Commit 7949df18 authored by Mark Chao's avatar Mark Chao

Check repo size for snippets

Add size checker access method to snippet.
Move size check logic to FOSS level.
parent d6aff9ca
...@@ -262,6 +262,15 @@ class Snippet < ApplicationRecord ...@@ -262,6 +262,15 @@ class Snippet < ApplicationRecord
@repository ||= Repository.new(full_path, self, shard: repository_storage, disk_path: disk_path, repo_type: Gitlab::GlRepository::SNIPPET) @repository ||= Repository.new(full_path, self, shard: repository_storage, disk_path: disk_path, repo_type: Gitlab::GlRepository::SNIPPET)
end end
def repository_size_checker
strong_memoize(:repository_size_checker) do
::Gitlab::RepositorySizeChecker.new(
current_size_proc: -> { repository._uncached_size.megabytes },
limit: Gitlab::CurrentSettings.snippet_size_limit
)
end
end
def storage def storage
@storage ||= Storage::Hashed.new(self, prefix: Storage::Hashed::SNIPPET_REPOSITORY_PATH_PREFIX) @storage ||= Storage::Hashed.new(self, prefix: Storage::Hashed::SNIPPET_REPOSITORY_PATH_PREFIX)
end end
......
...@@ -96,12 +96,6 @@ module EE ...@@ -96,12 +96,6 @@ module EE
actor == :geo actor == :geo
end end
def check_size_before_push!
if check_size_limit? && size_checker.above_size_limit?
raise ::Gitlab::GitAccess::ForbiddenError, size_checker.error_message.push_error
end
end
def check_if_license_blocks_changes! def check_if_license_blocks_changes!
if ::License.block_changes? if ::License.block_changes?
message = ::LicenseHelper.license_message(signed_in: true, is_admin: (user && user.admin?)) message = ::LicenseHelper.license_message(signed_in: true, is_admin: (user && user.admin?))
...@@ -109,65 +103,12 @@ module EE ...@@ -109,65 +103,12 @@ module EE
end end
end end
def check_push_size! override :check_size_limit?
return unless check_size_limit?
# If there are worktrees with a HEAD pointing to a non-existent object,
# calls to `git rev-list --all` will fail in git 2.15+. This should also
# clear stale lock files.
project.repository.clean_stale_repository_files
# Use #check_repository_disk_size to get correct push size whenever a lot of changes
# gets pushed at the same time containing the same blobs. This is only
# doable if GIT_OBJECT_DIRECTORY_RELATIVE env var is set and happens
# when git push comes from CLI (not via UI and API).
#
# Fallback to determining push size using the changes_list so we can still
# determine the push size if env var isn't set (e.g. changes are made
# via UI and API).
if check_quarantine_size?
check_repository_disk_size
else
check_changes_size
end
end
def check_quarantine_size?
git_env = ::Gitlab::Git::HookEnv.all(repository.gl_repository)
git_env['GIT_OBJECT_DIRECTORY_RELATIVE'].present?
end
def check_repository_disk_size
check_size_against_limit(project.repository.object_directory_size)
end
def check_changes_size
changes_size = 0
changes_list.each do |change|
changes_size += repository.new_blobs(change[:newrev]).sum(&:size) # rubocop: disable CodeReuse/ActiveRecord
check_size_against_limit(changes_size)
end
end
def check_size_against_limit(size)
if size_checker.changes_will_exceed_size_limit?(size)
raise ::Gitlab::GitAccess::ForbiddenError, size_checker.error_message.new_changes_error
end
end
def check_size_limit? def check_size_limit?
strong_memoize(:check_size_limit) do strong_memoize(:check_size_limit) do
size_checker.enabled? && size_checker.enabled? && super
changes_list.any? { |change| !::Gitlab::Git.blank_ref?(change[:newrev]) }
end end
end end
def size_checker
project.repository_size_checker
end
end end
end end
end end
...@@ -45,6 +45,8 @@ module Gitlab ...@@ -45,6 +45,8 @@ module Gitlab
attr_reader :actor, :project, :protocol, :authentication_abilities, :namespace_path, :repository_path, :redirected_path, :auth_result_type, :changes, :logger attr_reader :actor, :project, :protocol, :authentication_abilities, :namespace_path, :repository_path, :redirected_path, :auth_result_type, :changes, :logger
alias_method :container, :project
def initialize(actor, project, protocol, authentication_abilities:, namespace_path: nil, repository_path: nil, redirected_path: nil, auth_result_type: nil) def initialize(actor, project, protocol, authentication_abilities:, namespace_path: nil, repository_path: nil, redirected_path: nil, auth_result_type: nil)
@actor = actor @actor = actor
@project = project @project = project
...@@ -429,7 +431,72 @@ module Gitlab ...@@ -429,7 +431,72 @@ module Gitlab
end end
def repository def repository
project.repository container&.repository
end
def check_size_before_push!
if check_size_limit? && size_checker.above_size_limit?
raise ForbiddenError, size_checker.error_message.push_error
end
end
def check_push_size!
return unless check_size_limit?
# If there are worktrees with a HEAD pointing to a non-existent object,
# calls to `git rev-list --all` will fail in git 2.15+. This should also
# clear stale lock files.
repository.clean_stale_repository_files
# Use #check_repository_disk_size to get correct push size whenever a lot of changes
# gets pushed at the same time containing the same blobs. This is only
# doable if GIT_OBJECT_DIRECTORY_RELATIVE env var is set and happens
# when git push comes from CLI (not via UI and API).
#
# Fallback to determining push size using the changes_list so we can still
# determine the push size if env var isn't set (e.g. changes are made
# via UI and API).
if check_quarantine_size?
check_repository_disk_size
else
check_changes_size
end
end
def check_quarantine_size?
git_env = ::Gitlab::Git::HookEnv.all(repository.gl_repository)
git_env['GIT_OBJECT_DIRECTORY_RELATIVE'].present?
end
def check_repository_disk_size
check_size_against_limit(repository.object_directory_size)
end
def check_changes_size
changes_size = 0
changes_list.each do |change|
changes_size += repository.new_blobs(change[:newrev]).sum(&:size) # rubocop: disable CodeReuse/ActiveRecord
check_size_against_limit(changes_size)
end
end
def check_size_against_limit(size)
if size_checker.changes_will_exceed_size_limit?(size)
raise ForbiddenError, size_checker.error_message.new_changes_error
end
end
def check_size_limit?
strong_memoize(:check_size_limit) do
changes_list.any? { |change| !Gitlab::Git.blank_ref?(change[:newrev]) }
end
end
def size_checker
container.repository_size_checker
end end
end end
end end
......
...@@ -14,6 +14,8 @@ module Gitlab ...@@ -14,6 +14,8 @@ module Gitlab
attr_reader :snippet attr_reader :snippet
alias_method :container, :snippet
def initialize(actor, snippet, protocol, **kwargs) def initialize(actor, snippet, protocol, **kwargs)
@snippet = snippet @snippet = snippet
...@@ -53,11 +55,6 @@ module Gitlab ...@@ -53,11 +55,6 @@ module Gitlab
check_change_access! check_change_access!
end end
override :repository
def repository
snippet&.repository
end
def check_snippet_accessibility! def check_snippet_accessibility!
if snippet.blank? if snippet.blank?
raise NotFoundError, ERROR_MESSAGES[:snippet_not_found] raise NotFoundError, ERROR_MESSAGES[:snippet_not_found]
...@@ -89,11 +86,15 @@ module Gitlab ...@@ -89,11 +86,15 @@ module Gitlab
raise ForbiddenError, ERROR_MESSAGES[:update_snippet] raise ForbiddenError, ERROR_MESSAGES[:update_snippet]
end end
check_size_before_push!
changes_list.each do |change| changes_list.each do |change|
# If user does not have access to make at least one change, cancel all # If user does not have access to make at least one change, cancel all
# push by allowing the exception to bubble up # push by allowing the exception to bubble up
check_single_change_access(change) check_single_change_access(change)
end end
check_push_size!
end end
def check_single_change_access(change) def check_single_change_access(change)
......
...@@ -5,12 +5,14 @@ module Gitlab ...@@ -5,12 +5,14 @@ module Gitlab
class RepositorySizeChecker class RepositorySizeChecker
attr_reader :limit attr_reader :limit
# @param current_size_proc [Proc] returns repository size in bytes
def initialize(current_size_proc:, limit:, enabled: true) def initialize(current_size_proc:, limit:, enabled: true)
@current_size_proc = current_size_proc @current_size_proc = current_size_proc
@limit = limit @limit = limit
@enabled = enabled && limit != 0 @enabled = enabled && limit != 0
end end
# @return [Integer] bytes
def current_size def current_size
@current_size ||= @current_size_proc.call @current_size ||= @current_size_proc.call
end end
......
...@@ -11,6 +11,7 @@ describe Gitlab::GitAccessSnippet do ...@@ -11,6 +11,7 @@ describe Gitlab::GitAccessSnippet do
let_it_be(:user) { create(:user) } let_it_be(:user) { create(:user) }
let_it_be(:project) { create(:project, :public) } let_it_be(:project) { create(:project, :public) }
let_it_be(:snippet) { create(:project_snippet, :public, :repository, project: project) } let_it_be(:snippet) { create(:project_snippet, :public, :repository, project: project) }
let(:repository) { snippet.repository }
let(:actor) { user } let(:actor) { user }
let(:protocol) { 'ssh' } let(:protocol) { 'ssh' }
...@@ -211,6 +212,84 @@ describe Gitlab::GitAccessSnippet do ...@@ -211,6 +212,84 @@ describe Gitlab::GitAccessSnippet do
end end
end end
describe 'repository size restrictions' do
let(:snippet) { create(:personal_snippet, :public, :repository) }
let(:actor) { snippet.author }
let(:oldrev) { TestEnv::BRANCH_SHA["snippet/single-file"] }
let(:newrev) { TestEnv::BRANCH_SHA["snippet/edit-file"] }
let(:ref) { "refs/heads/snippet/edit-file" }
let(:changes) { "#{oldrev} #{newrev} #{ref}" }
shared_examples_for 'a push to repository already over the limit' do
it 'errs' do
expect(snippet.repository_size_checker).to receive(:above_size_limit?).and_return(true)
expect do
push_access_check
end.to raise_error(described_class::ForbiddenError, /Your push has been rejected/)
end
end
shared_examples_for 'a push to repository below the limit' do
it 'does not err' do
expect(snippet.repository_size_checker).to receive(:above_size_limit?).and_return(false)
expect(snippet.repository_size_checker)
.to receive(:changes_will_exceed_size_limit?)
.with(change_size)
.and_return(false)
expect { push_access_check }.not_to raise_error
end
end
shared_examples_for 'a push to repository to make it over the limit' do
it 'errs' do
expect(snippet.repository_size_checker).to receive(:above_size_limit?).and_return(false)
expect(snippet.repository_size_checker)
.to receive(:changes_will_exceed_size_limit?)
.with(change_size)
.and_return(true)
expect do
push_access_check
end.to raise_error(described_class::ForbiddenError, /Your push to this repository would cause it to exceed the size limit/)
end
end
context 'when GIT_OBJECT_DIRECTORY_RELATIVE env var is set' do
let(:change_size) { 100 }
before do
allow(Gitlab::Git::HookEnv)
.to receive(:all)
.with(repository.gl_repository)
.and_return({ 'GIT_OBJECT_DIRECTORY_RELATIVE' => 'objects' })
# Stub the object directory size to "simulate" quarantine size
allow(repository).to receive(:object_directory_size).and_return(change_size)
end
it_behaves_like 'a push to repository already over the limit'
it_behaves_like 'a push to repository below the limit'
it_behaves_like 'a push to repository to make it over the limit'
end
context 'when GIT_OBJECT_DIRECTORY_RELATIVE env var is not set' do
let(:change_size) { 200 }
before do
allow(snippet.repository).to receive(:new_blobs).and_return(
[double(:blob, size: change_size)]
)
end
it_behaves_like 'a push to repository already over the limit'
it_behaves_like 'a push to repository below the limit'
it_behaves_like 'a push to repository to make it over the limit'
end
end
private private
def raise_snippet_not_found def raise_snippet_not_found
......
...@@ -696,6 +696,23 @@ describe Snippet do ...@@ -696,6 +696,23 @@ describe Snippet do
end end
end end
describe '#repository_size_checker' do
subject { build(:personal_snippet) }
let(:checker) { subject.repository_size_checker }
let(:current_size) { 60 }
before do
allow(subject.repository).to receive(:_uncached_size).and_return(current_size)
end
it 'sets up size checker', :aggregate_failures do
expect(checker.current_size).to eq(current_size.megabytes)
expect(checker.limit).to eq(Gitlab::CurrentSettings.snippet_size_limit)
expect(checker.enabled?).to be_truthy
end
end
describe '#can_cache_field?' do describe '#can_cache_field?' do
using RSpec::Parameterized::TableSyntax using RSpec::Parameterized::TableSyntax
......
...@@ -60,11 +60,11 @@ module TestEnv ...@@ -60,11 +60,11 @@ module TestEnv
'merge-commit-analyze-before' => '1adbdef', 'merge-commit-analyze-before' => '1adbdef',
'merge-commit-analyze-side-branch' => '8a99451', 'merge-commit-analyze-side-branch' => '8a99451',
'merge-commit-analyze-after' => '646ece5', 'merge-commit-analyze-after' => '646ece5',
'snippet/single-file' => '43e4080', 'snippet/single-file' => '43e4080aaa14fc7d4b77ee1f5c9d067d5a7df10e',
'snippet/multiple-files' => 'b80faa8', 'snippet/multiple-files' => 'b80faa8c5b2b62f6489a0d84755580e927e1189b',
'snippet/rename-and-edit-file' => '220a1e4', 'snippet/rename-and-edit-file' => '220a1e4b4dff37feea0625a7947a4c60fbe78365',
'snippet/edit-file' => 'c2f074f', 'snippet/edit-file' => 'c2f074f4f26929c92795a75775af79a6ed6d8430',
'snippet/no-files' => '671aaa8', 'snippet/no-files' => '671aaa842a4875e5f30082d1ab6feda345fdb94d',
'2-mb-file' => 'bf12d25', '2-mb-file' => 'bf12d25',
'before-create-delete-modify-move' => '845009f', 'before-create-delete-modify-move' => '845009f',
'between-create-delete-modify-move' => '3f5f443', 'between-create-delete-modify-move' => '3f5f443',
......
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