Read about our upcoming Code of Conduct on this issue

Commit 0d2bdcf1 authored by Tiago Botelho's avatar Tiago Botelho
Browse files

Move new project on push logic to a service

parent d536c17fc4d0
......@@ -5,21 +5,13 @@ class Projects::GitHttpController < Projects::GitHttpClientController
rescue_from Gitlab::GitAccess::UnauthorizedError, with: :render_403
rescue_from Gitlab::GitAccess::NotFoundError, with: :render_404
rescue_from Gitlab::GitAccess::ProjectCreationError, with: :render_422
# GET /foo/bar.git/info/refs?service=git-upload-pack (git pull)
# GET /foo/bar.git/info/refs?service=git-receive-pack (git push)
def info_refs
log_user_activity if upload_pack?
if user && project.blank? && receive_pack?
@project = ::Projects::CreateService.new(user, project_params).execute
if @project.saved?
Gitlab::Checks::NewProject.new(user, @project, 'http').add_new_project_message
else
raise Gitlab::GitAccess::NotFoundError, 'Could not create project'
end
end
create_new_project if receive_pack? && project.blank?
render_ok
end
......@@ -31,8 +23,6 @@ def git_upload_pack
# POST /foo/bar.git/git-receive-pack" (git push)
def git_receive_pack
raise Gitlab::GitAccess::NotFoundError, 'Could not create project' unless project
render_ok
end
......@@ -58,6 +48,10 @@ def git_command
end
end
def create_new_project
@project = ::Projects::CreateFromPushService.new(user, params[:project_id], namespace, 'http').execute
end
def render_ok
set_workhorse_internal_api_content_type
render json: Gitlab::Workhorse.git_http_ok(repository, wiki?, user, action_name)
......@@ -71,6 +65,10 @@ def render_404(exception)
render plain: exception.message, status: :not_found
end
def render_422(exception)
render plain: exception.message, status: :unprocessable_entity
end
def access
@access ||= access_klass.new(access_actor, project, 'http', authentication_abilities: authentication_abilities, redirected_path: redirected_path, target_namespace: namespace)
end
......@@ -90,17 +88,8 @@ def access_klass
@access_klass ||= wiki? ? Gitlab::GitAccessWiki : Gitlab::GitAccess
end
def project_params
{
description: "",
path: Project.parse_project_id(params[:project_id]),
namespace_id: namespace&.id,
visibility_level: Gitlab::VisibilityLevel::PRIVATE.to_s
}
end
def namespace
@namespace ||= Namespace.find_by_path_or_name(params[:namespace_id])
@namespace ||= Namespace.find_by_full_path(params[:namespace_id])
end
def log_user_activity
......
......@@ -468,10 +468,6 @@ def cached_count
def group_ids
joins(:namespace).where(namespaces: { type: 'Group' }).select(:namespace_id)
end
def parse_project_id(project_id)
project_id.gsub("\.git", '')
end
end
# returns all ancestor-groups upto but excluding the given namespace
......
module Projects
class CreateFromPushService < BaseService
attr_reader :user, :project_path, :namespace, :protocol
def initialize(user, project_path, namespace, protocol)
@user = user
@project_path = project_path
@namespace = namespace
@protocol = protocol
end
def execute
return unless user
project = Projects::CreateService.new(user, project_params).execute
if project.saved?
Gitlab::Checks::ProjectCreated.new(user, project, protocol).add_project_created_message
else
raise Gitlab::GitAccess::ProjectCreationError, "Could not create project: #{project.errors.full_messages.join(', ')}"
end
project
end
private
def project_params
{
description: "",
path: project_path.gsub(/\.git$/, ''),
namespace_id: namespace&.id,
visibility_level: Gitlab::VisibilityLevel::PRIVATE.to_s
}
end
end
end
......@@ -29,10 +29,6 @@ def parse_env
{}
end
def receive_pack?
params[:action] == 'git-receive-pack'
end
def fix_git_env_repository_paths(env, repository_path)
if obj_dir_relative = env['GIT_OBJECT_DIRECTORY_RELATIVE'].presence
env['GIT_OBJECT_DIRECTORY'] = File.join(repository_path, obj_dir_relative)
......@@ -51,6 +47,10 @@ def log_user_activity(actor)
::Users::ActivityService.new(actor, 'Git SSH').execute if commands.include?(params[:action])
end
def receive_pack?
params[:action] == 'git-receive-pack'
end
def merge_request_urls
::MergeRequests::GetUrlsService.new(project).execute(params[:changes])
end
......@@ -64,29 +64,14 @@ def redis_ping
false
end
def project_params
{
description: "",
path: Project.parse_project_id(project_match[:project_id]),
namespace_id: project_namespace&.id,
visibility_level: Gitlab::VisibilityLevel::PRIVATE.to_s
}
def project_namespace
@project_namespace ||= project&.namespace || Namespace.find_by_full_path(project_match[:namespace_path])
end
private
def project_path_regex
@project_regex ||= /\A(?<namespace_id>#{Gitlab::PathRegex.full_namespace_route_regex})\/(?<project_id>#{Gitlab::PathRegex.project_git_route_regex})\z/.freeze
end
def project_match
@project_match ||= params[:project].match(project_path_regex)
end
def project_namespace
return unless project_match
@project_namespace ||= Namespace.find_by_path_or_name(project_match[:namespace_id])
@project_match ||= params[:project].match(Gitlab::PathRegex.full_project_git_path_regex)
end
# rubocop:disable Gitlab/ModuleWithInstanceVariables
......
......@@ -51,13 +51,11 @@ class Internal < Grape::API
return { status: false, message: e.message }
end
if user && project.blank? && receive_pack?
@project = ::Projects::CreateService.new(user, project_params).execute
if @project.saved?
Gitlab::Checks::NewProject.new(user, @project, protocol).add_new_project_message
else
return { status: false, message: "Could not create project" }
if receive_pack? && project.blank?
begin
@project = ::Projects::CreateFromPushService.new(user, project_match[:project_path], project_namespace, protocol).execute
rescue Gitlab::GitAccess::ProjectCreationError => e
return { status: false, message: e.message }
end
end
......@@ -218,10 +216,10 @@ class Internal < Grape::API
# key could be used
if user
redirect_message = Gitlab::Checks::ProjectMoved.fetch_redirect_message(user.id, project.id)
new_project_message = Gitlab::Checks::NewProject.fetch_new_project_message(user.id, project.id)
project_created_message = Gitlab::Checks::ProjectCreated.fetch_project_created_message(user.id, project.id)
output[:redirected_message] = redirect_message if redirect_message
output[:new_project_message] = new_project_message if new_project_message
output[:project_created_message] = project_created_message if project_created_message
end
output
......
module Gitlab
module Checks
class NewProject
NEW_PROJECT = "new_project".freeze
class ProjectCreated
PROJECT_CREATED = "project_created".freeze
def initialize(user, project, protocol)
@user = user
......@@ -9,26 +9,26 @@ def initialize(user, project, protocol)
@protocol = protocol
end
def self.fetch_new_project_message(user_id, project_id)
new_project_key = new_project_message_key(user_id, project_id)
def self.fetch_project_created_message(user_id, project_id)
project_created_key = project_created_message_key(user_id, project_id)
Gitlab::Redis::SharedState.with do |redis|
message = redis.get(new_project_key)
redis.del(new_project_key)
message = redis.get(project_created_key)
redis.del(project_created_key)
message
end
end
def add_new_project_message
def add_project_created_message
return unless user.present? && project.present?
Gitlab::Redis::SharedState.with do |redis|
key = self.class.new_project_message_key(user.id, project.id)
redis.setex(key, 5.minutes, new_project_message)
key = self.class.project_created_message_key(user.id, project.id)
redis.setex(key, 5.minutes, project_created_message)
end
end
def new_project_message
def project_created_message
<<~MESSAGE.strip_heredoc
The private project #{project.full_path} was created.
......@@ -46,8 +46,8 @@ def new_project_message
attr_reader :project, :user, :protocol
def self.new_project_message_key(user_id, project_id)
"#{NEW_PROJECT}:#{user_id}:#{project_id}"
def self.project_created_message_key(user_id, project_id)
"#{PROJECT_CREATED}:#{user_id}:#{project_id}"
end
def project_url
......
......@@ -4,6 +4,7 @@ module Gitlab
class GitAccess
UnauthorizedError = Class.new(StandardError)
NotFoundError = Class.new(StandardError)
ProjectCreationError = Class.new(StandardError)
ProjectMovedError = Class.new(NotFoundError)
ERROR_MESSAGES = {
......@@ -13,13 +14,13 @@ class GitAccess
'This deploy key does not have write access to this project.',
no_repo: 'A repository for this project does not exist yet.',
project_not_found: 'The project you were looking for could not be found.',
namespace_not_found: 'The namespace you were looking for could not be found.',
account_blocked: 'Your account has been blocked.',
command_not_allowed: "The command you're trying to execute is not allowed.",
upload_pack_disabled_over_http: 'Pulling over HTTP is not allowed.',
receive_pack_disabled_over_http: 'Pushing over HTTP is not allowed.',
read_only: 'The repository is temporarily read-only. Please try again later.',
cannot_push_to_read_only: "You can't push code to a read-only GitLab instance."
cannot_push_to_read_only: "You can't push code to a read-only GitLab instance.",
namespace_not_found: 'The namespace you were looking for could not be found.'
}.freeze
DOWNLOAD_COMMANDS = %w{ git-upload-pack git-upload-archive }.freeze
......@@ -52,7 +53,7 @@ def check(cmd, changes)
check_download_access!
when *PUSH_COMMANDS
check_push_access!(cmd, changes)
check_namespace_accessibility!(cmd)
check_namespace_accessibility!
end
true
......@@ -148,7 +149,7 @@ def check_repository_existence!(cmd)
end
end
def check_namespace_accessibility!(cmd)
def check_namespace_accessibility!
return unless project.blank?
unless target_namespace
......@@ -248,7 +249,7 @@ def can_read_project?
end
def can_create_project_in_namespace?(cmd)
return false unless PUSH_COMMANDS.include?(cmd) && target_namespace
return false unless push?(cmd) && target_namespace
user.can?(:create_projects, target_namespace)
end
......@@ -265,6 +266,10 @@ def receive_pack?(command)
command == 'git-receive-pack'
end
def push?(cmd)
PUSH_COMMANDS.include?(cmd)
end
def upload_pack_disabled_over_http?
!Gitlab.config.gitlab_shell.upload_pack
end
......
......@@ -25,10 +25,6 @@ def check_single_change_access(change)
true
end
def check_repository_creation!(cmd)
# Method not used in wiki
end
def push_to_read_only_message
ERROR_MESSAGES[:read_only]
end
......
......@@ -187,6 +187,10 @@ def full_project_path_regex
@full_project_path_regex ||= %r{\A#{full_namespace_route_regex}/#{project_route_regex}/\z}
end
def full_project_git_path_regex
@full_project_git_path_regex ||= /\A(\/|)(?<namespace_path>#{full_namespace_route_regex})\/(?<project_path>#{project_git_route_regex})\z/.freeze
end
def full_namespace_format_regex
@namespace_format_regex ||= /A#{FULL_NAMESPACE_FORMAT_REGEX}\z/.freeze
end
......
require 'rails_helper'
describe Gitlab::Checks::NewProject, :clean_gitlab_redis_shared_state do
let(:user) { create(:user) }
let(:project) { create(:project) }
describe '.fetch_new_project_message' do
context 'with a new project message queue' do
let(:new_project) { described_class.new(user, project, 'http') }
before do
new_project.add_new_project_message
end
it 'returns new project message' do
expect(described_class.fetch_new_project_message(user.id, project.id)).to eq(new_project.new_project_message)
end
it 'deletes the new project message from redis' do
expect(Gitlab::Redis::SharedState.with { |redis| redis.get("new_project:#{user.id}:#{project.id}") }).not_to be_nil
described_class.fetch_new_project_message(user.id, project.id)
expect(Gitlab::Redis::SharedState.with { |redis| redis.get("new_project:#{user.id}:#{project.id}") }).to be_nil
end
end
context 'with no new project message queue' do
it 'returns nil' do
expect(described_class.fetch_new_project_message(1, 2)).to be_nil
end
end
end
describe '#add_new_project_message' do
it 'queues a new project message' do
new_project = described_class.new(user, project, 'http')
expect(new_project.add_new_project_message).to eq('OK')
end
it 'handles anonymous push' do
new_project = described_class.new(user, nil, 'http')
expect(new_project.add_new_project_message).to be_nil
end
end
end
require 'rails_helper'
describe Gitlab::Checks::ProjectCreated, :clean_gitlab_redis_shared_state do
let(:user) { create(:user) }
let(:project) { create(:project) }
describe '.fetch_project_created_message' do
context 'with a project created message queue' do
let(:project_created) { described_class.new(user, project, 'http') }
before do
project_created.add_project_created_message
end
it 'returns project created message' do
expect(described_class.fetch_project_created_message(user.id, project.id)).to eq(project_created.project_created_message)
end
it 'deletes the project created message from redis' do
expect(Gitlab::Redis::SharedState.with { |redis| redis.get("project_created:#{user.id}:#{project.id}") }).not_to be_nil
described_class.fetch_project_created_message(user.id, project.id)
expect(Gitlab::Redis::SharedState.with { |redis| redis.get("project_created:#{user.id}:#{project.id}") }).to be_nil
end
end
context 'with no project created message queue' do
it 'returns nil' do
expect(described_class.fetch_project_created_message(1, 2)).to be_nil
end
end
end
describe '#add_project_created_message' do
it 'queues a project created message' do
project_created = described_class.new(user, project, 'http')
expect(project_created.add_project_created_message).to eq('OK')
end
it 'handles anonymous push' do
project_created = described_class.new(user, nil, 'http')
expect(project_created.add_project_created_message).to be_nil
end
end
end
......@@ -335,7 +335,7 @@ def disable_protocol(protocol)
end
end
describe '#check_namespace_accessibility!' do
describe '#check_namespace_existence!' do
context 'when project exists' do
context 'when user can pull or push' do
before do
......@@ -838,18 +838,11 @@ def raise_unauthorized(message)
end
def raise_not_found
raise_error(Gitlab::GitAccess::NotFoundError,
Gitlab::GitAccess::ERROR_MESSAGES[:project_not_found])
raise_error(Gitlab::GitAccess::NotFoundError, Gitlab::GitAccess::ERROR_MESSAGES[:project_not_found])
end
def raise_namespace_not_found
raise_error(Gitlab::GitAccess::NotFoundError,
Gitlab::GitAccess::ERROR_MESSAGES[:namespace_not_found])
end
def raise_project_create
raise_error(Gitlab::GitAccess::NotFoundError,
Gitlab::GitAccess::ERROR_MESSAGES[:create])
raise_error(Gitlab::GitAccess::NotFoundError, Gitlab::GitAccess::ERROR_MESSAGES[:namespace_not_found])
end
def build_authentication_abilities
......
......@@ -1079,12 +1079,6 @@ def create_pipeline
end
end
describe '.parse_project_id' do
it 'removes .git from the project_id' do
expect(described_class.parse_project_id('new-project.git')).to eq('new-project')
end
end
context 'repository storage by default' do
let(:project) { create(:project) }
......
......@@ -366,6 +366,17 @@
end
end
context 'project as /namespace/project' do
it do
push(key, project_with_repo_path('/' + project.full_path))
expect(response).to have_gitlab_http_status(200)
expect(json_response["status"]).to be_truthy
expect(json_response["repository_path"]).to eq(project.repository.path_to_repo)
expect(json_response["gl_repository"]).to eq("project-#{project.id}")
end
end
context 'project as namespace/project' do
it do
push(key, project_with_repo_path(project.full_path))
......@@ -823,14 +834,14 @@
context 'with new project data' do
it 'returns new project message on the response' do
new_project = Gitlab::Checks::NewProject.new(user, project, 'http')
new_project.add_new_project_message
project_created = Gitlab::Checks::ProjectCreated.new(user, project, 'http')
project_created.add_project_created_message
post api("/internal/post_receive"), valid_params
expect(response).to have_gitlab_http_status(200)
expect(json_response["new_project_message"]).to be_present
expect(json_response["new_project_message"]).to eq(new_project.new_project_message)
expect(json_response["project_created_message"]).to be_present
expect(json_response["project_created_message"]).to eq(project_created.project_created_message)
end
end
......
require 'spec_helper'
describe Projects::CreateFromPushService do
let(:user) { create(:user) }
let(:project_path) { "nonexist" }
let(:namespace) { user&.namespace }
let(:protocol) { 'http' }
subject { described_class.new(user, project_path, namespace, protocol) }
it 'creates project' do
expect_any_instance_of(Projects::CreateService).to receive(:execute).and_call_original
expect { subject.execute }.to change { Project.count }.by(1)
end
it 'raises project creation error when project creation fails' do
allow_any_instance_of(Project).to receive(:saved?).and_return(false)
expect { subject.execute }.to raise_error(Gitlab::GitAccess::ProjectCreationError)
end
context 'when user is nil' do
let(:user) { nil }
subject { described_class.new(user, project_path, namespace, cmd, protocol) }
it 'returns nil' do
expect_any_instance_of(Projects::CreateService).not_to receive(:execute)
expect(subject.execute).to be_nil
end
end
end
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