summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--app/models/commit.rb2
-rw-r--r--app/models/gpg_signature.rb4
-rw-r--r--app/services/git_push_service.rb15
-rw-r--r--app/workers/create_gpg_signature_worker.rb8
-rw-r--r--config/initializers/active_record_array_type_casting.rb20
-rw-r--r--lib/gitlab/git/commit.rb19
-rw-r--r--lib/gitlab/gpg/commit.rb38
-rw-r--r--lib/gitlab/gpg/invalid_gpg_signature_updater.rb4
-rw-r--r--spec/lib/gitlab/gpg/commit_spec.rb71
-rw-r--r--spec/lib/gitlab/gpg/invalid_gpg_signature_updater_spec.rb23
-rw-r--r--spec/services/git_push_service_spec.rb34
-rw-r--r--spec/workers/create_gpg_signature_worker_spec.rb26
12 files changed, 160 insertions, 104 deletions
diff --git a/app/models/commit.rb b/app/models/commit.rb
index 638fddc5d3d..be9a56c190c 100644
--- a/app/models/commit.rb
+++ b/app/models/commit.rb
@@ -383,6 +383,6 @@ class Commit
end
def gpg_commit
- @gpg_commit ||= Gitlab::Gpg::Commit.new(self)
+ @gpg_commit ||= Gitlab::Gpg::Commit.for_commit(self)
end
end
diff --git a/app/models/gpg_signature.rb b/app/models/gpg_signature.rb
index 1ac0e123ff1..50fb35c77ec 100644
--- a/app/models/gpg_signature.rb
+++ b/app/models/gpg_signature.rb
@@ -18,4 +18,8 @@ class GpgSignature < ActiveRecord::Base
def commit
project.commit(commit_sha)
end
+
+ def gpg_commit
+ Gitlab::Gpg::Commit.new(project, commit_sha)
+ end
end
diff --git a/app/services/git_push_service.rb b/app/services/git_push_service.rb
index ada2b64a3a6..e81a56672e2 100644
--- a/app/services/git_push_service.rb
+++ b/app/services/git_push_service.rb
@@ -90,8 +90,19 @@ class GitPushService < BaseService
end
def update_signatures
- @push_commits.each do |commit|
- CreateGpgSignatureWorker.perform_async(commit.sha, @project.id)
+ commit_shas = @push_commits.last(PROCESS_COMMIT_LIMIT).map(&:sha)
+
+ return if commit_shas.empty?
+
+ shas_with_cached_signatures = GpgSignature.where(commit_sha: commit_shas).pluck(:commit_sha)
+ commit_shas -= shas_with_cached_signatures
+
+ return if commit_shas.empty?
+
+ commit_shas = Gitlab::Git::Commit.shas_with_signatures(project.repository, commit_shas)
+
+ commit_shas.each do |sha|
+ CreateGpgSignatureWorker.perform_async(sha, project.id)
end
end
diff --git a/app/workers/create_gpg_signature_worker.rb b/app/workers/create_gpg_signature_worker.rb
index 4f47717ff69..f34dff2d656 100644
--- a/app/workers/create_gpg_signature_worker.rb
+++ b/app/workers/create_gpg_signature_worker.rb
@@ -4,13 +4,9 @@ class CreateGpgSignatureWorker
def perform(commit_sha, project_id)
project = Project.find_by(id: project_id)
-
return unless project
- commit = project.commit(commit_sha)
-
- return unless commit
-
- commit.signature
+ # This calculates and caches the signature in the database
+ Gitlab::Gpg::Commit.new(project, commit_sha).signature
end
end
diff --git a/config/initializers/active_record_array_type_casting.rb b/config/initializers/active_record_array_type_casting.rb
new file mode 100644
index 00000000000..d94d592add6
--- /dev/null
+++ b/config/initializers/active_record_array_type_casting.rb
@@ -0,0 +1,20 @@
+module ActiveRecord
+ class PredicateBuilder
+ class ArrayHandler
+ module TypeCasting
+ def call(attribute, value)
+ # This is necessary because by default ActiveRecord does not respect
+ # custom type definitions (like our `ShaAttribute`) when providing an
+ # array in `where`, like in `where(commit_sha: [sha1, sha2, sha3])`.
+ model = attribute.relation&.engine
+ type = model.user_provided_columns[attribute.name] if model
+ value = value.map { |value| type.type_cast_for_database(value) } if type
+
+ super(attribute, value)
+ end
+ end
+
+ prepend TypeCasting
+ end
+ end
+end
diff --git a/lib/gitlab/git/commit.rb b/lib/gitlab/git/commit.rb
index fd4dfdb09a2..a499bbc6266 100644
--- a/lib/gitlab/git/commit.rb
+++ b/lib/gitlab/git/commit.rb
@@ -210,6 +210,16 @@ module Gitlab
@rugged_sort_types.fetch(sort_type, Rugged::SORT_NONE)
end
+
+ def shas_with_signatures(repository, shas)
+ shas.select do |sha|
+ begin
+ Rugged::Commit.extract_signature(repository.rugged, sha)
+ rescue Rugged::OdbError
+ false
+ end
+ end
+ end
end
def initialize(repository, raw_commit, head = nil)
@@ -335,15 +345,6 @@ module Gitlab
parent_ids.map { |oid| self.class.find(@repository, oid) }.compact
end
- # Get the gpg signature of this commit.
- #
- # Ex.
- # commit.signature(repo)
- #
- def signature(repo)
- Rugged::Commit.extract_signature(repo.rugged, sha)
- end
-
def stats
Gitlab::Git::CommitStats.new(self)
end
diff --git a/lib/gitlab/gpg/commit.rb b/lib/gitlab/gpg/commit.rb
index 55428b85207..606c7576f70 100644
--- a/lib/gitlab/gpg/commit.rb
+++ b/lib/gitlab/gpg/commit.rb
@@ -1,12 +1,20 @@
module Gitlab
module Gpg
class Commit
- attr_reader :commit
+ def self.for_commit(commit)
+ new(commit.project, commit.sha)
+ end
- def initialize(commit)
- @commit = commit
+ def initialize(project, sha)
+ @project = project
+ @sha = sha
- @signature_text, @signed_text = commit.raw.signature(commit.project.repository)
+ @signature_text, @signed_text =
+ begin
+ Rugged::Commit.extract_signature(project.repository.rugged, sha)
+ rescue Rugged::OdbError
+ nil
+ end
end
def has_signature?
@@ -16,18 +24,20 @@ module Gitlab
def signature
return unless has_signature?
- cached_signature = GpgSignature.find_by(commit_sha: commit.sha)
- return cached_signature if cached_signature.present?
+ return @signature if @signature
- using_keychain do |gpg_key|
- create_cached_signature!(gpg_key)
- end
+ cached_signature = GpgSignature.find_by(commit_sha: @sha)
+ return @signature = cached_signature if cached_signature.present?
+
+ @signature = create_cached_signature!
end
def update_signature!(cached_signature)
using_keychain do |gpg_key|
cached_signature.update_attributes!(attributes(gpg_key))
end
+
+ @signature = cached_signature
end
private
@@ -55,16 +65,18 @@ module Gitlab
end
end
- def create_cached_signature!(gpg_key)
- GpgSignature.create!(attributes(gpg_key))
+ def create_cached_signature!
+ using_keychain do |gpg_key|
+ GpgSignature.create!(attributes(gpg_key))
+ end
end
def attributes(gpg_key)
user_infos = user_infos(gpg_key)
{
- commit_sha: commit.sha,
- project: commit.project,
+ commit_sha: @sha,
+ project: @project,
gpg_key: gpg_key,
gpg_key_primary_keyid: gpg_key&.primary_keyid || verified_signature.fingerprint,
gpg_key_user_name: user_infos[:name],
diff --git a/lib/gitlab/gpg/invalid_gpg_signature_updater.rb b/lib/gitlab/gpg/invalid_gpg_signature_updater.rb
index 3bb491120ba..a525ee7a9ee 100644
--- a/lib/gitlab/gpg/invalid_gpg_signature_updater.rb
+++ b/lib/gitlab/gpg/invalid_gpg_signature_updater.rb
@@ -10,9 +10,7 @@ module Gitlab
.select(:id, :commit_sha, :project_id)
.where('gpg_key_id IS NULL OR valid_signature = ?', false)
.where(gpg_key_primary_keyid: @gpg_key.primary_keyid)
- .find_each do |gpg_signature|
- Gitlab::Gpg::Commit.new(gpg_signature.commit).update_signature!(gpg_signature)
- end
+ .find_each { |sig| sig.gpg_commit.update_signature!(sig) }
end
end
end
diff --git a/spec/lib/gitlab/gpg/commit_spec.rb b/spec/lib/gitlab/gpg/commit_spec.rb
index ddb8dd9f0f4..e521fcc6dc1 100644
--- a/spec/lib/gitlab/gpg/commit_spec.rb
+++ b/spec/lib/gitlab/gpg/commit_spec.rb
@@ -1,13 +1,13 @@
require 'rails_helper'
-RSpec.describe Gitlab::Gpg::Commit do
+describe Gitlab::Gpg::Commit do
describe '#signature' do
let!(:project) { create :project, :repository, path: 'sample-project' }
let!(:commit_sha) { '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33' }
- context 'unisgned commit' do
+ context 'unsigned commit' do
it 'returns nil' do
- expect(described_class.new(project.commit).signature).to be_nil
+ expect(described_class.new(project, commit_sha).signature).to be_nil
end
end
@@ -16,18 +16,19 @@ RSpec.describe Gitlab::Gpg::Commit do
create :gpg_key, key: GpgHelpers::User1.public_key, user: create(:user, email: GpgHelpers::User1.emails.first)
end
- let!(:commit) do
- raw_commit = double(:raw_commit, signature: [
- GpgHelpers::User1.signed_commit_signature,
- GpgHelpers::User1.signed_commit_base_data
- ], sha: commit_sha)
- allow(raw_commit).to receive :save!
-
- create :commit, git_commit: raw_commit, project: project
+ before do
+ allow(Rugged::Commit).to receive(:extract_signature)
+ .with(Rugged::Repository, commit_sha)
+ .and_return(
+ [
+ GpgHelpers::User1.signed_commit_signature,
+ GpgHelpers::User1.signed_commit_base_data
+ ]
+ )
end
it 'returns a valid signature' do
- expect(described_class.new(commit).signature).to have_attributes(
+ expect(described_class.new(project, commit_sha).signature).to have_attributes(
commit_sha: commit_sha,
project: project,
gpg_key: gpg_key,
@@ -39,7 +40,7 @@ RSpec.describe Gitlab::Gpg::Commit do
end
it 'returns the cached signature on second call' do
- gpg_commit = described_class.new(commit)
+ gpg_commit = described_class.new(project, commit_sha)
expect(gpg_commit).to receive(:using_keychain).and_call_original
gpg_commit.signature
@@ -53,18 +54,19 @@ RSpec.describe Gitlab::Gpg::Commit do
context 'known but unverified public key' do
let!(:gpg_key) { create :gpg_key, key: GpgHelpers::User1.public_key }
- let!(:commit) do
- raw_commit = double(:raw_commit, signature: [
- GpgHelpers::User1.signed_commit_signature,
- GpgHelpers::User1.signed_commit_base_data
- ], sha: commit_sha)
- allow(raw_commit).to receive :save!
-
- create :commit, git_commit: raw_commit, project: project
+ before do
+ allow(Rugged::Commit).to receive(:extract_signature)
+ .with(Rugged::Repository, commit_sha)
+ .and_return(
+ [
+ GpgHelpers::User1.signed_commit_signature,
+ GpgHelpers::User1.signed_commit_base_data
+ ]
+ )
end
it 'returns an invalid signature' do
- expect(described_class.new(commit).signature).to have_attributes(
+ expect(described_class.new(project, commit_sha).signature).to have_attributes(
commit_sha: commit_sha,
project: project,
gpg_key: gpg_key,
@@ -76,7 +78,7 @@ RSpec.describe Gitlab::Gpg::Commit do
end
it 'returns the cached signature on second call' do
- gpg_commit = described_class.new(commit)
+ gpg_commit = described_class.new(project, commit_sha)
expect(gpg_commit).to receive(:using_keychain).and_call_original
gpg_commit.signature
@@ -88,20 +90,19 @@ RSpec.describe Gitlab::Gpg::Commit do
end
context 'unknown public key' do
- let!(:commit) do
- raw_commit = double(:raw_commit, signature: [
- GpgHelpers::User1.signed_commit_signature,
- GpgHelpers::User1.signed_commit_base_data
- ], sha: commit_sha)
- allow(raw_commit).to receive :save!
-
- create :commit,
- git_commit: raw_commit,
- project: project
+ before do
+ allow(Rugged::Commit).to receive(:extract_signature)
+ .with(Rugged::Repository, commit_sha)
+ .and_return(
+ [
+ GpgHelpers::User1.signed_commit_signature,
+ GpgHelpers::User1.signed_commit_base_data
+ ]
+ )
end
it 'returns an invalid signature' do
- expect(described_class.new(commit).signature).to have_attributes(
+ expect(described_class.new(project, commit_sha).signature).to have_attributes(
commit_sha: commit_sha,
project: project,
gpg_key: nil,
@@ -113,7 +114,7 @@ RSpec.describe Gitlab::Gpg::Commit do
end
it 'returns the cached signature on second call' do
- gpg_commit = described_class.new(commit)
+ gpg_commit = described_class.new(project, commit_sha)
expect(gpg_commit).to receive(:using_keychain).and_call_original
gpg_commit.signature
diff --git a/spec/lib/gitlab/gpg/invalid_gpg_signature_updater_spec.rb b/spec/lib/gitlab/gpg/invalid_gpg_signature_updater_spec.rb
index c4e04ee46a2..4de4419de27 100644
--- a/spec/lib/gitlab/gpg/invalid_gpg_signature_updater_spec.rb
+++ b/spec/lib/gitlab/gpg/invalid_gpg_signature_updater_spec.rb
@@ -4,23 +4,16 @@ RSpec.describe Gitlab::Gpg::InvalidGpgSignatureUpdater do
describe '#run' do
let!(:commit_sha) { '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33' }
let!(:project) { create :project, :repository, path: 'sample-project' }
- let!(:raw_commit) do
- raw_commit = double(:raw_commit, signature: [
- GpgHelpers::User1.signed_commit_signature,
- GpgHelpers::User1.signed_commit_base_data
- ], sha: commit_sha)
-
- allow(raw_commit).to receive :save!
-
- raw_commit
- end
-
- let!(:commit) do
- create :commit, git_commit: raw_commit, project: project
- end
before do
- allow_any_instance_of(Project).to receive(:commit).and_return(commit)
+ allow(Rugged::Commit).to receive(:extract_signature)
+ .with(Rugged::Repository, commit_sha)
+ .and_return(
+ [
+ GpgHelpers::User1.signed_commit_signature,
+ GpgHelpers::User1.signed_commit_base_data
+ ]
+ )
end
context 'gpg signature did have an associated gpg key which was removed later' do
diff --git a/spec/services/git_push_service_spec.rb b/spec/services/git_push_service_spec.rb
index 8485605b398..e3c1bdce300 100644
--- a/spec/services/git_push_service_spec.rb
+++ b/spec/services/git_push_service_spec.rb
@@ -688,10 +688,38 @@ describe GitPushService, services: true do
)
end
- it 'calls CreateGpgSignatureWorker.perform_async for each commit' do
- expect(CreateGpgSignatureWorker).to receive(:perform_async).with(sample_commit.id, project.id)
+ context 'when the commit has a signature' do
+ context 'when the signature is already cached' do
+ before do
+ create(:gpg_signature, commit_sha: sample_commit.id)
+ end
- execute_service(project, user, oldrev, newrev, ref)
+ it 'does not queue a CreateGpgSignatureWorker' do
+ expect(CreateGpgSignatureWorker).not_to receive(:perform_async).with(sample_commit.id, project.id)
+
+ execute_service(project, user, oldrev, newrev, ref)
+ end
+ end
+
+ context 'when the signature is not yet cached' do
+ it 'queues a CreateGpgSignatureWorker' do
+ expect(CreateGpgSignatureWorker).to receive(:perform_async).with(sample_commit.id, project.id)
+
+ execute_service(project, user, oldrev, newrev, ref)
+ end
+ end
+ end
+
+ context 'when the commit does not have a signature' do
+ before do
+ allow(Gitlab::Git::Commit).to receive(:shas_with_signatures).with(project.repository, [sample_commit.id]).and_return([])
+ end
+
+ it 'does not queue a CreateGpgSignatureWorker' do
+ expect(CreateGpgSignatureWorker).not_to receive(:perform_async).with(sample_commit.id, project.id)
+
+ execute_service(project, user, oldrev, newrev, ref)
+ end
end
end
diff --git a/spec/workers/create_gpg_signature_worker_spec.rb b/spec/workers/create_gpg_signature_worker_spec.rb
index c6a17d77d73..54978baca88 100644
--- a/spec/workers/create_gpg_signature_worker_spec.rb
+++ b/spec/workers/create_gpg_signature_worker_spec.rb
@@ -1,34 +1,26 @@
require 'spec_helper'
describe CreateGpgSignatureWorker do
+ let(:project) { create(:project, :repository) }
+
context 'when GpgKey is found' do
- it 'calls Commit#signature' do
- commit_sha = '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33'
- project = create :project
- commit = instance_double(Commit)
+ let(:commit_sha) { '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33' }
- allow(Project).to receive(:find_by).with(id: project.id).and_return(project)
- allow(project).to receive(:commit).with(commit_sha).and_return(commit)
+ it 'calls Gitlab::Gpg::Commit#signature' do
+ expect(Gitlab::Gpg::Commit).to receive(:new).with(project, commit_sha).and_call_original
- expect(commit).to receive(:signature)
+ expect_any_instance_of(Gitlab::Gpg::Commit).to receive(:signature)
described_class.new.perform(commit_sha, project.id)
end
end
context 'when Commit is not found' do
- let(:nonexisting_commit_sha) { 'bogus' }
- let(:project) { create :project }
+ let(:nonexisting_commit_sha) { '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a34' }
it 'does not raise errors' do
expect { described_class.new.perform(nonexisting_commit_sha, project.id) }.not_to raise_error
end
-
- it 'does not call Commit#signature' do
- expect_any_instance_of(Commit).not_to receive(:signature)
-
- described_class.new.perform(nonexisting_commit_sha, project.id)
- end
end
context 'when Project is not found' do
@@ -38,8 +30,8 @@ describe CreateGpgSignatureWorker do
expect { described_class.new.perform(anything, nonexisting_project_id) }.not_to raise_error
end
- it 'does not call Commit#signature' do
- expect_any_instance_of(Commit).not_to receive(:signature)
+ it 'does not call Gitlab::Gpg::Commit#signature' do
+ expect_any_instance_of(Gitlab::Gpg::Commit).not_to receive(:signature)
described_class.new.perform(anything, nonexisting_project_id)
end
View Lorry Controller Status