summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--app/assets/javascripts/packages_and_registries/infrastructure_registry/components/terraform_installation.vue3
-rw-r--r--app/models/concerns/has_repository.rb5
-rw-r--r--app/models/lfs_download_object.rb16
-rw-r--r--app/models/project.rb17
-rw-r--r--app/models/repository.rb13
-rw-r--r--app/models/user.rb2
-rw-r--r--app/services/projects/lfs_pointers/lfs_download_link_list_service.rb2
-rw-r--r--app/services/projects/lfs_pointers/lfs_download_service.rb14
-rw-r--r--app/uploaders/object_storage.rb5
-rw-r--r--spec/frontend/packages_and_registries/infrastructure_registry/components/__snapshots__/terraform_installation_spec.js.snap2
-rw-r--r--spec/frontend/packages_and_registries/infrastructure_registry/components/terraform_installation_spec.js2
-rw-r--r--spec/models/lfs_download_object_spec.rb50
-rw-r--r--spec/models/project_spec.rb26
-rw-r--r--spec/models/repository_spec.rb32
-rw-r--r--spec/models/user_spec.rb1
-rw-r--r--spec/services/projects/lfs_pointers/lfs_download_link_list_service_spec.rb13
-rw-r--r--spec/services/projects/lfs_pointers/lfs_download_service_spec.rb19
-rw-r--r--spec/support/shared_examples/models/concerns/has_repository_shared_examples.rb16
-rw-r--r--spec/uploaders/object_storage_spec.rb6
19 files changed, 189 insertions, 55 deletions
diff --git a/app/assets/javascripts/packages_and_registries/infrastructure_registry/components/terraform_installation.vue b/app/assets/javascripts/packages_and_registries/infrastructure_registry/components/terraform_installation.vue
index c19ff3b4293..c62bf7fb722 100644
--- a/app/assets/javascripts/packages_and_registries/infrastructure_registry/components/terraform_installation.vue
+++ b/app/assets/javascripts/packages_and_registries/infrastructure_registry/components/terraform_installation.vue
@@ -14,8 +14,7 @@ export default {
computed: {
...mapState(['packageEntity', 'terraformHelpPath', 'gitlabHost', 'projectPath']),
provisionInstructions() {
- // eslint-disable-next-line @gitlab/require-i18n-strings
- return `module "${this.packageEntity.name}" {
+ return `module "my_module_name" {
source = "${this.gitlabHost}/${this.projectPath}/${this.packageEntity.name}"
version = "${this.packageEntity.version}"
}`;
diff --git a/app/models/concerns/has_repository.rb b/app/models/concerns/has_repository.rb
index 33f6904bc91..1b4c590694a 100644
--- a/app/models/concerns/has_repository.rb
+++ b/app/models/concerns/has_repository.rb
@@ -14,6 +14,7 @@ module HasRepository
include Gitlab::Utils::StrongMemoize
delegate :base_dir, :disk_path, to: :storage
+ delegate :change_head, to: :repository
def valid_repo?
repository.exists?
@@ -117,4 +118,8 @@ module HasRepository
def repository_size_checker
raise NotImplementedError
end
+
+ def after_repository_change_head
+ reload_default_branch
+ end
end
diff --git a/app/models/lfs_download_object.rb b/app/models/lfs_download_object.rb
index 6383f95d546..319499fd1b7 100644
--- a/app/models/lfs_download_object.rb
+++ b/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
diff --git a/app/models/project.rb b/app/models/project.rb
index 1f8e8b81015..95ba0973321 100644
--- a/app/models/project.rb
+++ b/app/models/project.rb
@@ -1661,18 +1661,11 @@ class Project < ApplicationRecord
:visibility_level
end
- def change_head(branch)
- if repository.branch_exists?(branch)
- repository.before_change_head
- repository.raw_repository.write_ref('HEAD', "refs/heads/#{branch}")
- repository.copy_gitattributes(branch)
- repository.after_change_head
- ProjectCacheWorker.perform_async(self.id, [], [:commit_count])
- reload_default_branch
- else
- errors.add(:base, _("Could not change HEAD: branch '%{branch}' does not exist") % { branch: branch })
- false
- end
+ override :after_repository_change_head
+ def after_repository_change_head
+ ProjectCacheWorker.perform_async(self.id, [], [:commit_count])
+
+ super
end
def forked_from?(other_project)
diff --git a/app/models/repository.rb b/app/models/repository.rb
index 1bd61fe48cb..a77aaf02e06 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -466,6 +466,7 @@ class Repository
# Runs code after the HEAD of a repository is changed.
def after_change_head
expire_all_method_caches
+ container.after_repository_change_head
end
# Runs code after a new commit has been pushed.
@@ -1142,6 +1143,18 @@ class Repository
Gitlab::CurrentSettings.pick_repository_storage
end
+ def change_head(branch)
+ if branch_exists?(branch)
+ before_change_head
+ raw_repository.write_ref('HEAD', "refs/heads/#{branch}")
+ copy_gitattributes(branch)
+ after_change_head
+ else
+ container.errors.add(:base, _("Could not change HEAD: branch '%{branch}' does not exist") % { branch: branch })
+ false
+ end
+ end
+
private
# TODO Genericize finder, later split this on finders by Ref or Oid
diff --git a/app/models/user.rb b/app/models/user.rb
index 52bf9149ee2..ce702131151 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -2095,7 +2095,7 @@ class User < ApplicationRecord
end
def check_username_format
- return if username.blank? || Mime::EXTENSION_LOOKUP.keys.none? { |type| username.end_with?(type) }
+ return if username.blank? || Mime::EXTENSION_LOOKUP.keys.none? { |type| username.end_with?(".#{type}") }
errors.add(:username, _('ending with MIME type format is not allowed.'))
end
diff --git a/app/services/projects/lfs_pointers/lfs_download_link_list_service.rb b/app/services/projects/lfs_pointers/lfs_download_link_list_service.rb
index efd410088ab..c82ed97203f 100644
--- a/app/services/projects/lfs_pointers/lfs_download_link_list_service.rb
+++ b/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.")
diff --git a/app/services/projects/lfs_pointers/lfs_download_service.rb b/app/services/projects/lfs_pointers/lfs_download_service.rb
index 525f8a25d04..9e2edf7c4ef 100644
--- a/app/services/projects/lfs_pointers/lfs_download_service.rb
+++ b/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
diff --git a/app/uploaders/object_storage.rb b/app/uploaders/object_storage.rb
index be5839b7ec5..1d56cddca63 100644
--- a/app/uploaders/object_storage.rb
+++ b/app/uploaders/object_storage.rb
@@ -283,8 +283,9 @@ module ObjectStorage
if file_storage?
IO.copy_stream(path, file)
else
- streamer = lambda { |chunk, _, _| file.write(chunk) }
- Excon.get(url, response_block: streamer)
+ Faraday.get(url) do |req|
+ req.options.on_data = proc { |chunk, _| file.write(chunk) }
+ end
end
file.seek(0, IO::SEEK_SET)
diff --git a/spec/frontend/packages_and_registries/infrastructure_registry/components/__snapshots__/terraform_installation_spec.js.snap b/spec/frontend/packages_and_registries/infrastructure_registry/components/__snapshots__/terraform_installation_spec.js.snap
index 16d148bbea4..03236737572 100644
--- a/spec/frontend/packages_and_registries/infrastructure_registry/components/__snapshots__/terraform_installation_spec.js.snap
+++ b/spec/frontend/packages_and_registries/infrastructure_registry/components/__snapshots__/terraform_installation_spec.js.snap
@@ -10,7 +10,7 @@ exports[`TerraformInstallation renders all the messages 1`] = `
<code-instruction-stub
copytext="Copy Terraform Command"
- instruction="module \\"Test/system-22\\" {
+ instruction="module \\"my_module_name\\" {
source = \\"bar.dev/foo/Test/system-22\\"
version = \\"0.1\\"
}"
diff --git a/spec/frontend/packages_and_registries/infrastructure_registry/components/terraform_installation_spec.js b/spec/frontend/packages_and_registries/infrastructure_registry/components/terraform_installation_spec.js
index d527587da16..ee1548ed5eb 100644
--- a/spec/frontend/packages_and_registries/infrastructure_registry/components/terraform_installation_spec.js
+++ b/spec/frontend/packages_and_registries/infrastructure_registry/components/terraform_installation_spec.js
@@ -42,7 +42,7 @@ describe('TerraformInstallation', () => {
describe('installation commands', () => {
it('renders the correct command', () => {
expect(findCodeInstructions().at(0).props('instruction')).toMatchInlineSnapshot(`
- "module \\"Test/system-22\\" {
+ "module \\"my_module_name\\" {
source = \\"bar.dev/foo/Test/system-22\\"
version = \\"0.1\\"
}"
diff --git a/spec/models/lfs_download_object_spec.rb b/spec/models/lfs_download_object_spec.rb
index d1c323cd177..d82e432b7d6 100644
--- a/spec/models/lfs_download_object_spec.rb
+++ b/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
diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb
index 7eb02749f72..144b00e1d2e 100644
--- a/spec/models/project_spec.rb
+++ b/spec/models/project_spec.rb
@@ -3122,35 +3122,19 @@ RSpec.describe Project, factory_default: :keep do
end
end
- describe '#change_head' do
- let_it_be(:project) { create(:project, :repository) }
-
- it 'returns error if branch does not exist' do
- expect(project.change_head('unexisted-branch')).to be false
- expect(project.errors.size).to eq(1)
- end
-
- it 'calls the before_change_head and after_change_head methods' do
- expect(project.repository).to receive(:before_change_head)
- expect(project.repository).to receive(:after_change_head)
-
- project.change_head(project.default_branch)
- end
+ describe '#after_repository_change_head' do
+ let_it_be(:project) { create(:project) }
it 'updates commit count' do
expect(ProjectCacheWorker).to receive(:perform_async).with(project.id, [], [:commit_count])
- project.change_head(project.default_branch)
- end
-
- it 'copies the gitattributes' do
- expect(project.repository).to receive(:copy_gitattributes).with(project.default_branch)
- project.change_head(project.default_branch)
+ project.after_repository_change_head
end
it 'reloads the default branch' do
expect(project).to receive(:reload_default_branch)
- project.change_head(project.default_branch)
+
+ project.after_repository_change_head
end
end
diff --git a/spec/models/repository_spec.rb b/spec/models/repository_spec.rb
index b6f09babb4b..c896b6c0c6c 100644
--- a/spec/models/repository_spec.rb
+++ b/spec/models/repository_spec.rb
@@ -2138,6 +2138,12 @@ RSpec.describe Repository do
repository.after_change_head
end
+
+ it 'calls after_repository_change_head on container' do
+ expect(repository.container).to receive(:after_repository_change_head)
+
+ repository.after_change_head
+ end
end
describe '#expires_caches_for_tags' do
@@ -3261,4 +3267,30 @@ RSpec.describe Repository do
settings.save!
end
end
+
+ describe '#change_head' do
+ let(:branch) { repository.container.default_branch }
+
+ it 'adds an error to container if branch does not exist' do
+ expect(repository.change_head('unexisted-branch')).to be false
+ expect(repository.container.errors.size).to eq(1)
+ end
+
+ it 'calls the before_change_head and after_change_head methods' do
+ expect(repository).to receive(:before_change_head)
+ expect(repository).to receive(:after_change_head)
+
+ repository.change_head(branch)
+ end
+
+ it 'copies the gitattributes' do
+ expect(repository).to receive(:copy_gitattributes).with(branch)
+ repository.change_head(branch)
+ end
+
+ it 'reloads the default branch' do
+ expect(repository.container).to receive(:reload_default_branch)
+ repository.change_head(branch)
+ end
+ end
end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 2185df0609e..e86a9c262d8 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -394,6 +394,7 @@ RSpec.describe User do
expect(user).not_to be_valid
expect(user.errors.full_messages).to include('Username ending with MIME type format is not allowed.')
+ expect(build(:user, username: "test#{type}")).to be_valid
end
end
diff --git a/spec/services/projects/lfs_pointers/lfs_download_link_list_service_spec.rb b/spec/services/projects/lfs_pointers/lfs_download_link_list_service_spec.rb
index 66a450bd734..047ebe65dff 100644
--- a/spec/services/projects/lfs_pointers/lfs_download_link_list_service_spec.rb
+++ b/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))
diff --git a/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb b/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb
index 1b829df6e6a..1fb6dae0c08 100644
--- a/spec/services/projects/lfs_pointers/lfs_download_service_spec.rb
+++ b/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
diff --git a/spec/support/shared_examples/models/concerns/has_repository_shared_examples.rb b/spec/support/shared_examples/models/concerns/has_repository_shared_examples.rb
index 1be4d9b80a4..a403a27adef 100644
--- a/spec/support/shared_examples/models/concerns/has_repository_shared_examples.rb
+++ b/spec/support/shared_examples/models/concerns/has_repository_shared_examples.rb
@@ -152,4 +152,20 @@ RSpec.shared_examples 'model with repository' do
it { is_expected.to respond_to(:disk_path) }
it { is_expected.to respond_to(:gitlab_shell) }
end
+
+ describe '#change_head' do
+ it 'delegates #change_head to repository' do
+ expect(stubbed_container.repository).to receive(:change_head).with('foo')
+
+ stubbed_container.change_head('foo')
+ end
+ end
+
+ describe '#after_repository_change_head' do
+ it 'calls #reload_default_branch' do
+ expect(stubbed_container).to receive(:reload_default_branch)
+
+ stubbed_container.after_repository_change_head
+ end
+ end
end
diff --git a/spec/uploaders/object_storage_spec.rb b/spec/uploaders/object_storage_spec.rb
index a1d8695a8c9..13f70e3f85b 100644
--- a/spec/uploaders/object_storage_spec.rb
+++ b/spec/uploaders/object_storage_spec.rb
@@ -222,7 +222,11 @@ RSpec.describe ObjectStorage do
before do
stub_artifacts_object_storage
- stub_request(:get, %r{s3.amazonaws.com/#{uploader.path}}).to_return(status: 200, body: '')
+
+ # We need to check the Host header not including the port because AWS does not accept
+ stub_request(:get, %r{s3.amazonaws.com/#{uploader.path}})
+ .with { |request| !request.headers['Host'].to_s.include?(':443') }
+ .to_return(status: 200, body: '')
end
it "returns the file" do