Only create commit GPG signature when necessary
This commit is contained in:
parent
4a2a6d521a
commit
ba7251fefd
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
|
@ -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
|
||||
|
|
|
@ -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],
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
||||
|
|
|
@ -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
|
||||
|
|
Loading…
Reference in New Issue