Commit 80155d6d authored by Ash McKenzie's avatar Ash McKenzie

Project#lfs_http_url_to_repo can be overriden

- Project#lfs_http_url_to_repo() returns #http_url_to_repo()
- EE::Project#lfs_http_url_to_repo() returns #http_url_to_repo() if isn't a Geo secondary with a primary, otherwise, returns Geo primary project HTTP URL
- Use Project#lfs_http_url_to_repo() in API /internal/lfs_authenticate
parent ff42f3e9
......@@ -1141,6 +1141,11 @@ class Project < ActiveRecord::Base
"#{web_url}.git"
end
# Is overriden in EE
def lfs_http_url_to_repo(_)
http_url_to_repo
end
def forked?
fork_network && fork_network.root_project != self
end
......
......@@ -10,6 +10,9 @@ module EE
extend ::Gitlab::Utils::Override
extend ::Gitlab::Cache::RequestCache
include ::Gitlab::Utils::StrongMemoize
include ::EE::GitlabRoutingHelper
GIT_LFS_DOWNLOAD_OPERATION = 'download'.freeze
prepended do
include Elastic::ProjectsSearch
......@@ -488,6 +491,14 @@ module EE
webide_pipelines.running_or_pending.for_user(user)
end
override :lfs_http_url_to_repo
def lfs_http_url_to_repo(operation)
return super unless ::Gitlab::Geo.secondary_with_primary?
return super if operation == GIT_LFS_DOWNLOAD_OPERATION # download always comes from secondary
geo_primary_http_url_to_repo(self)
end
private
def set_override_pull_mirror_available
......
---
title: 'Geo: Fix push to secondary over SSH for LFS'
merge_request: 8044
author:
type: fixed
......@@ -2,6 +2,7 @@ require 'spec_helper'
describe Project do
include ExternalAuthorizationServiceHelpers
include ::EE::GeoHelpers
using RSpec::Parameterized::TableSyntax
describe 'associations' do
......@@ -1366,6 +1367,78 @@ describe Project do
end
end
describe '#lfs_http_url_to_repo' do
let(:project) { create(:project) }
let(:project_path) { "#{Gitlab::Routing.url_helpers.project_path(project)}.git" }
let(:primary_base_host) { 'primary.geo' }
let(:primary_base_url) { "http://#{primary_base_host}" }
let(:primary_url) { "#{primary_base_url}#{project_path}" }
context 'with a Geo setup that is a primary' do
let(:primary_node) { create(:geo_node, url: primary_base_url) }
before do
stub_current_geo_node(primary_node)
stub_default_url_options(primary_base_host)
end
context 'for an upload operation' do
it 'returns the project HTTP URL for the primary' do
expect(project.lfs_http_url_to_repo('upload')).to eq(primary_url)
end
end
end
context 'with a Geo setup that is a secondary' do
let(:secondary_base_host) { 'secondary.geo' }
let(:secondary_base_url) { "http://#{secondary_base_host}" }
let(:secondary_node) { create(:geo_node, url: secondary_base_url) }
let(:secondary_url) { "#{secondary_base_url}#{project_path}" }
before do
stub_current_geo_node(secondary_node)
stub_default_url_options(current_rails_hostname)
end
context 'and has a primary' do
let(:primary_node) { create(:geo_node, url: primary_base_url) }
context 'for an upload operation' do
let(:current_rails_hostname) { primary_base_host }
it 'returns the project HTTP URL for the primary' do
expect(project.lfs_http_url_to_repo('upload')).to eq(primary_url)
end
end
context 'for a download operation' do
let(:current_rails_hostname) { secondary_base_host }
it 'returns the project HTTP URL for the secondary' do
expect(project.lfs_http_url_to_repo('download')).to eq(secondary_url)
end
end
end
context 'without a primary' do
let(:current_rails_hostname) { secondary_base_host }
it 'returns the project HTTP URL for the secondary' do
expect(project.lfs_http_url_to_repo('operation_that_doesnt_matter')).to eq(secondary_url)
end
end
end
context 'without a Geo setup' do
it 'returns the project HTTP URL for the main node' do
project_url = "#{Gitlab::Routing.url_helpers.project_url(project)}.git"
expect(project.lfs_http_url_to_repo('operation_that_doesnt_matter')).to eq(project_url)
end
end
end
describe '#add_import_job' do
let(:project) { create(:project) }
......@@ -1469,4 +1542,13 @@ describe Project do
subject
end
end
# Despite stubbing the current node as the primary or secondary, the
# behaviour for EE::Project#lfs_http_url_to_repo() is to call
# Project#lfs_http_url_to_repo() which does not have a Geo context.
def stub_default_url_options(host)
allow(Rails.application.routes)
.to receive(:default_url_options)
.and_return(host: host)
end
end
......@@ -122,7 +122,7 @@ module API
{
username: token_handler.actor_name,
lfs_token: token_handler.token,
repository_http_path: project.http_url_to_repo
repository_http_path: project.lfs_http_url_to_repo(params[:operation])
}
end
# rubocop: enable CodeReuse/ActiveRecord
......
......@@ -2991,6 +2991,17 @@ describe Project do
end
end
describe '#lfs_http_url_to_repo' do
let(:project) { create(:project) }
it 'returns the url to the repo without a username' do
lfs_http_url_to_repo = project.lfs_http_url_to_repo('operation_that_doesnt_matter')
expect(lfs_http_url_to_repo).to eq("#{project.web_url}.git")
expect(lfs_http_url_to_repo).not_to include('@')
end
end
describe '#pipeline_status' do
let(:project) { create(:project, :repository) }
it 'builds a pipeline status' do
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment