Commit a7b7a225 authored by James Edwards-Jones's avatar James Edwards-Jones

Prevent git push when LFS objects are missing

parent ca049902
---
title: Prevent git push when LFS objects are missing
merge_request: 13837
author:
type: added
...@@ -12,7 +12,8 @@ module Gitlab ...@@ -12,7 +12,8 @@ module Gitlab
change_existing_tags: 'You are not allowed to change existing tags on this project.', change_existing_tags: 'You are not allowed to change existing tags on this project.',
update_protected_tag: 'Protected tags cannot be updated.', update_protected_tag: 'Protected tags cannot be updated.',
delete_protected_tag: 'Protected tags cannot be deleted.', delete_protected_tag: 'Protected tags cannot be deleted.',
create_protected_tag: 'You are not allowed to create this tag as it is protected.' create_protected_tag: 'You are not allowed to create this tag as it is protected.',
lfs_objects_missing: 'LFS objects are missing. Ensure LFS is properly set up or try a manual "git lfs push --all".'
}.freeze }.freeze
attr_reader :user_access, :project, :skip_authorization, :protocol attr_reader :user_access, :project, :skip_authorization, :protocol
...@@ -36,6 +37,7 @@ module Gitlab ...@@ -36,6 +37,7 @@ module Gitlab
push_checks push_checks
branch_checks branch_checks
tag_checks tag_checks
lfs_objects_exist_check
true true
end end
...@@ -136,6 +138,14 @@ module Gitlab ...@@ -136,6 +138,14 @@ module Gitlab
def matching_merge_request? def matching_merge_request?
Checks::MatchingMergeRequest.new(@newrev, @branch_name, @project).match? Checks::MatchingMergeRequest.new(@newrev, @branch_name, @project).match?
end end
def lfs_objects_exist_check
lfs_check = Checks::LfsIntegrity.new(project, @newrev)
if lfs_check.objects_missing?
raise GitAccess::UnauthorizedError, ERROR_MESSAGES[:lfs_objects_missing]
end
end
end end
end end
end end
module Gitlab
module Checks
class LfsIntegrity
REV_LIST_OBJECT_LIMIT = 2_000
def initialize(project, newrev)
@project = project
@newrev = newrev
end
def objects_missing?
return false unless @newrev && @project.lfs_enabled?
new_lfs_pointers = Gitlab::Git::LfsChanges.new(@project.repository, @newrev).new_pointers(object_limit: REV_LIST_OBJECT_LIMIT)
return false unless new_lfs_pointers.present?
existing_count = @project.lfs_objects.where(oid: new_lfs_pointers.map(&:lfs_oid)).count
existing_count != new_lfs_pointers.count
end
end
end
end
...@@ -11,13 +11,19 @@ describe Gitlab::Checks::ChangeAccess do ...@@ -11,13 +11,19 @@ describe Gitlab::Checks::ChangeAccess do
let(:changes) { { oldrev: oldrev, newrev: newrev, ref: ref } } let(:changes) { { oldrev: oldrev, newrev: newrev, ref: ref } }
let(:protocol) { 'ssh' } let(:protocol) { 'ssh' }
subject do let(:change_access) do
described_class.new( described_class.new(
changes, changes,
project: project, project: project,
user_access: user_access, user_access: user_access,
protocol: protocol protocol: protocol
).exec )
end
subject do
# TODO: Replace use of `subject` with `subject.exec`
# Then rename change_access back to subject
change_access.exec
end end
before do before do
...@@ -163,5 +169,50 @@ describe Gitlab::Checks::ChangeAccess do ...@@ -163,5 +169,50 @@ describe Gitlab::Checks::ChangeAccess do
end end
end end
end end
context 'LFS integrity check' do
let(:blob_object) { project.repository.blob_at_branch('lfs', 'files/lfs/lfs_object.iso') }
before do
allow_any_instance_of(Gitlab::Git::RevList).to receive(:new_objects) do |&lazy_block|
lazy_block.call([blob_object.id])
end
end
context 'with LFS not enabled' do
it 'skips integrity check' do
expect_any_instance_of(Gitlab::Git::RevList).not_to receive(:new_objects)
change_access.exec
end
end
context 'with LFS enabled' do
before do
allow(project).to receive(:lfs_enabled?).and_return(true)
end
context 'deletion' do
let(:changes) { { oldrev: oldrev, ref: ref } }
it 'skips integrity check' do
expect_any_instance_of(Gitlab::Git::RevList).not_to receive(:new_objects)
change_access.exec
end
end
it 'fails if any LFS blobs are missing' do
expect { change_access.exec }.to raise_error(Gitlab::GitAccess::UnauthorizedError, /LFS objects are missing/)
end
it 'succeeds if LFS objects have already been uploaded' do
lfs_object = create(:lfs_object, oid: blob_object.lfs_oid)
create(:lfs_objects_project, project: project, lfs_object: lfs_object)
expect { change_access.exec }.not_to raise_error
end
end
end
end 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