diff options
author | James Edwards-Jones <jedwardsjones@gitlab.com> | 2017-08-25 02:30:12 +0100 |
---|---|---|
committer | James Edwards-Jones <jedwardsjones@gitlab.com> | 2017-11-06 11:15:20 +0000 |
commit | 96d4f0c62fce0df3dac6f8de15d65a5ea570047c (patch) | |
tree | e8095f60d6ce738e671b74060ac1b25f5ecfd136 | |
parent | ca049902dc7dad6e6177b05c8e3dc74c00487d27 (diff) | |
download | gitlab-ce-96d4f0c62fce0df3dac6f8de15d65a5ea570047c.tar.gz |
Prevent git push when LFS objects are missing
-rw-r--r-- | lib/gitlab/checks/change_access.rb | 12 | ||||
-rw-r--r-- | lib/gitlab/checks/lfs_integrity.rb | 24 | ||||
-rw-r--r-- | spec/lib/gitlab/checks/change_access_spec.rb | 55 |
3 files changed, 88 insertions, 3 deletions
diff --git a/lib/gitlab/checks/change_access.rb b/lib/gitlab/checks/change_access.rb index b6805230348..ef92fc5a0a0 100644 --- a/lib/gitlab/checks/change_access.rb +++ b/lib/gitlab/checks/change_access.rb @@ -12,7 +12,8 @@ module Gitlab change_existing_tags: 'You are not allowed to change existing tags on this project.', update_protected_tag: 'Protected tags cannot be updated.', 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 attr_reader :user_access, :project, :skip_authorization, :protocol @@ -36,6 +37,7 @@ module Gitlab push_checks branch_checks tag_checks + lfs_objects_exist_check true end @@ -136,6 +138,14 @@ module Gitlab def matching_merge_request? Checks::MatchingMergeRequest.new(@newrev, @branch_name, @project).match? 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 diff --git a/lib/gitlab/checks/lfs_integrity.rb b/lib/gitlab/checks/lfs_integrity.rb new file mode 100644 index 00000000000..68870bd6e95 --- /dev/null +++ b/lib/gitlab/checks/lfs_integrity.rb @@ -0,0 +1,24 @@ +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 unless @newrev && @project.lfs_enabled? + + new_lfs_pointers = Gitlab::Git::LfsChanges.new(@project.repository, @newrev).new_pointers(object_limit: REV_LIST_OBJECT_LIMIT) + + return 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 diff --git a/spec/lib/gitlab/checks/change_access_spec.rb b/spec/lib/gitlab/checks/change_access_spec.rb index 6c25b7349e1..3a7d3f2176b 100644 --- a/spec/lib/gitlab/checks/change_access_spec.rb +++ b/spec/lib/gitlab/checks/change_access_spec.rb @@ -11,13 +11,19 @@ describe Gitlab::Checks::ChangeAccess do let(:changes) { { oldrev: oldrev, newrev: newrev, ref: ref } } let(:protocol) { 'ssh' } - subject do + let(:change_access) do described_class.new( changes, project: project, user_access: user_access, 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 before do @@ -163,5 +169,50 @@ describe Gitlab::Checks::ChangeAccess do 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 |