Merge branch 'bwill/ssh-signatures/models' into 'master'

Add models for SSH signed commits

See merge request gitlab-org/gitlab!87962

app/models/commit_signatures/ssh_signature.rb

+# frozen_string_literal: true
+
+module CommitSignatures
+  class SshSignature < ApplicationRecord
+    include CommitSignature
+
+    belongs_to :key, optional: false
+  end
+end

db/docs/ssh_signatures.yml

+---
+table_name: ssh_signatures
+classes:
+- CommitSignatures::SshSignature
+feature_categories:
+- source_code_management
+description: >
+  The verification status for commits which are signed by SSH keys. The actual signature
+  is part of the commit body and is stored in Gitaly.
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/87962
+milestone: '15.1'

db/migrate/20220518183504_create_ssh_signatures.rb

+# frozen_string_literal: true
+
+class CreateSshSignatures < Gitlab::Database::Migration[2.0]
+  def change
+    create_table :ssh_signatures do |t|
+      t.timestamps_with_timezone null: false
+      t.bigint :project_id, null: false, index: true
+      t.bigint :key_id, null: false, index: true
+      t.integer :verification_status, default: 0, null: false, limit: 2
+      t.binary :commit_sha, null: false, index: { unique: true }
+    end
+  end
+end

db/migrate/20220518183548_add_projects_relation_to_ssh_signatures.rb

+# frozen_string_literal: true
+
+class AddProjectsRelationToSshSignatures < Gitlab::Database::Migration[2.0]
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_foreign_key :ssh_signatures, :projects, column: :project_id, on_delete: :cascade
+  end
+
+  def down
+    with_lock_retries do
+      remove_foreign_key :ssh_signatures, column: :project_id
+    end
+  end
+end

db/migrate/20220520143105_add_keys_relation_to_ssh_signatures.rb

+# frozen_string_literal: true
+
+class AddKeysRelationToSshSignatures < Gitlab::Database::Migration[2.0]
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_foreign_key :ssh_signatures, :keys, column: :key_id, on_delete: :cascade
+  end
+
+  def down
+    with_lock_retries do
+      remove_foreign_key :ssh_signatures, column: :key_id
+    end
+  end
+end

db/schema_migrations/20220518183504

+925cd6bbfc67d0f5748c48b960ef1f932370fe078a979440b6bb32d049c2a9a8
\ No newline at end of file

db/schema_migrations/20220518183548

+a79526f7eb59fc93d66ff1a58471c9a3de27f8e620b5f3d4a255c88687a5bf2a
\ No newline at end of file

db/schema_migrations/20220520143105

+f31157879c1d7e2f08a63b4c68ed0353fd6df1e885cb7f3838aba7e1c782394c
\ No newline at end of file

db/structure.sql

@@ -20872,6 +20872,25 @@ CREATE SEQUENCE sprints_id_seq
 ALTER SEQUENCE sprints_id_seq OWNED BY sprints.id;
+CREATE TABLE ssh_signatures (
+    id bigint NOT NULL,
+    created_at timestamp with time zone NOT NULL,
+    updated_at timestamp with time zone NOT NULL,
+    project_id bigint NOT NULL,
+    key_id bigint NOT NULL,
+    verification_status smallint DEFAULT 0 NOT NULL,
+    commit_sha bytea NOT NULL
+);
+
+CREATE SEQUENCE ssh_signatures_id_seq
+    START WITH 1
+    INCREMENT BY 1
+    NO MINVALUE
+    NO MAXVALUE
+    CACHE 1;
+
+ALTER SEQUENCE ssh_signatures_id_seq OWNED BY ssh_signatures.id;
+
 CREATE TABLE status_check_responses (
     id bigint NOT NULL,
     merge_request_id bigint NOT NULL,
@@ -23245,6 +23264,8 @@ ALTER TABLE ONLY spam_logs ALTER COLUMN id SET DEFAULT nextval('spam_logs_id_seq
 ALTER TABLE ONLY sprints ALTER COLUMN id SET DEFAULT nextval('sprints_id_seq'::regclass);
+ALTER TABLE ONLY ssh_signatures ALTER COLUMN id SET DEFAULT nextval('ssh_signatures_id_seq'::regclass);
+
 ALTER TABLE ONLY status_check_responses ALTER COLUMN id SET DEFAULT nextval('status_check_responses_id_seq'::regclass);
 ALTER TABLE ONLY status_page_published_incidents ALTER COLUMN id SET DEFAULT nextval('status_page_published_incidents_id_seq'::regclass);
@@ -25454,6 +25475,9 @@ ALTER TABLE ONLY spam_logs
 ALTER TABLE ONLY sprints
     ADD CONSTRAINT sprints_pkey PRIMARY KEY (id);
+ALTER TABLE ONLY ssh_signatures
+    ADD CONSTRAINT ssh_signatures_pkey PRIMARY KEY (id);
+
 ALTER TABLE ONLY status_check_responses
     ADD CONSTRAINT status_check_responses_pkey PRIMARY KEY (id);
@@ -29319,6 +29343,12 @@ CREATE INDEX index_sprints_on_title ON sprints USING btree (title);
 CREATE INDEX index_sprints_on_title_trigram ON sprints USING gin (title gin_trgm_ops);
+CREATE UNIQUE INDEX index_ssh_signatures_on_commit_sha ON ssh_signatures USING btree (commit_sha);
+
+CREATE INDEX index_ssh_signatures_on_key_id ON ssh_signatures USING btree (key_id);
+
+CREATE INDEX index_ssh_signatures_on_project_id ON ssh_signatures USING btree (project_id);
+
 CREATE INDEX index_status_check_responses_on_external_approval_rule_id ON status_check_responses USING btree (external_approval_rule_id);
 CREATE INDEX index_status_check_responses_on_external_status_check_id ON status_check_responses USING btree (external_status_check_id);
@@ -31588,6 +31618,9 @@ ALTER TABLE ONLY issue_customer_relations_contacts
 ALTER TABLE ONLY vulnerabilities
     ADD CONSTRAINT fk_7c5bb22a22 FOREIGN KEY (due_date_sourcing_milestone_id) REFERENCES milestones(id) ON DELETE SET NULL;
+ALTER TABLE ONLY ssh_signatures
+    ADD CONSTRAINT fk_7d2f93996c FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY labels
     ADD CONSTRAINT fk_7de4989a69 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE;
@@ -32020,6 +32053,9 @@ ALTER TABLE ONLY epics
 ALTER TABLE ONLY boards
     ADD CONSTRAINT fk_f15266b5f9 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE;
+ALTER TABLE ONLY ssh_signatures
+    ADD CONSTRAINT fk_f177ea6aa5 FOREIGN KEY (key_id) REFERENCES keys(id) ON DELETE CASCADE;
+
 ALTER TABLE ONLY ci_pipeline_variables
     ADD CONSTRAINT fk_f29c5f4380 FOREIGN KEY (pipeline_id) REFERENCES ci_pipelines(id) ON DELETE CASCADE;

lib/gitlab/database/gitlab_schemas.yml

@@ -492,6 +492,7 @@ software_license_policies: :gitlab_main
 software_licenses: :gitlab_main
 spam_logs: :gitlab_main
 sprints: :gitlab_main
+ssh_signatures: :gitlab_main
 status_check_responses: :gitlab_main
 status_page_published_incidents: :gitlab_main
 status_page_settings: :gitlab_main

spec/factories/commit_signature/ssh_signature.rb

+# frozen_string_literal: true
+
+FactoryBot.define do
+  factory :ssh_signature, class: 'CommitSignatures::SshSignature' do
+    commit_sha { Digest::SHA1.hexdigest(SecureRandom.hex) }
+    project
+    key
+    verification_status { :verified }
+  end
+end

spec/models/commit_signatures/gpg_signature_spec.rb

@@ -3,17 +3,26 @@
 require 'spec_helper'
 RSpec.describe CommitSignatures::GpgSignature do
+  # This commit is seeded from https://gitlab.com/gitlab-org/gitlab-test
+  # For instructions on how to add more seed data, see the project README
   let(:commit_sha) { '0beec7b5ea3f0fdbc95d0dd47f3c5bc275da8a33' }
   let!(:project) { create(:project, :repository, path: 'sample-project') }
   let!(:commit) { create(:commit, project: project, sha: commit_sha) }
-  let(:gpg_signature) { create(:gpg_signature, commit_sha: commit_sha) }
+  let(:signature) { create(:gpg_signature, commit_sha: commit_sha) }
   let(:gpg_key) { create(:gpg_key) }
   let(:gpg_key_subkey) { create(:gpg_key_subkey) }
+  let(:attributes) do
+    {
+      commit_sha: commit_sha,
+      project: project,
+      gpg_key_primary_keyid: gpg_key.keyid
+    }
+  end
   it_behaves_like 'having unique enum values'
+  it_behaves_like 'commit signature'
   describe 'associations' do
-    it { is_expected.to belong_to(:project).required }
     it { is_expected.to belong_to(:gpg_key) }
     it { is_expected.to belong_to(:gpg_key_subkey) }
   end
@@ -22,104 +31,56 @@
     subject { described_class.new }
     it { is_expected.to validate_presence_of(:commit_sha) }
-    it { is_expected.to validate_presence_of(:project_id) }
     it { is_expected.to validate_presence_of(:gpg_key_primary_keyid) }
   end
-  describe '.safe_create!' do
-    let(:attributes) do
-      {
-        commit_sha: commit_sha,
-        project: project,
-        gpg_key_primary_keyid: gpg_key.keyid
-      }
-    end
-
-    it 'finds a signature by commit sha if it existed' do
-      gpg_signature
-
-      expect(described_class.safe_create!(commit_sha: commit_sha)).to eq(gpg_signature)
-    end
-
-    it 'creates a new signature if it was not found' do
-      expect { described_class.safe_create!(attributes) }.to change { described_class.count }.by(1)
-    end
-
-    it 'assigns the correct attributes when creating' do
-      signature = described_class.safe_create!(attributes)
-
-      expect(signature.project).to eq(project)
-      expect(signature.commit_sha).to eq(commit_sha)
-      expect(signature.gpg_key_primary_keyid).to eq(gpg_key.keyid)
-    end
-
-    it 'does not raise an error in case of a race condition' do
-      expect(described_class).to receive(:find_by).and_return(nil, double(described_class, persisted?: true))
-
-      expect(described_class).to receive(:create).and_raise(ActiveRecord::RecordNotUnique)
-      allow(described_class).to receive(:create).and_call_original
-
-      described_class.safe_create!(attributes)
-    end
-  end
-
   describe '.by_commit_sha scope' do
     let(:gpg_key) { create(:gpg_key, key: GpgHelpers::User2.public_key) }
     let!(:another_gpg_signature) { create(:gpg_signature, gpg_key: gpg_key) }
     it 'returns all gpg signatures by sha' do
-      expect(described_class.by_commit_sha(commit_sha)).to eq([gpg_signature])
+      expect(described_class.by_commit_sha(commit_sha)).to match_array([signature])
       expect(
         described_class.by_commit_sha([commit_sha, another_gpg_signature.commit_sha])
-      ).to contain_exactly(gpg_signature, another_gpg_signature)
-    end
-  end
-
-  describe '#commit' do
-    it 'fetches the commit through the project' do
-      expect_next_instance_of(Project) do |instance|
-        expect(instance).to receive(:commit).with(commit_sha).and_return(commit)
-      end
-
-      gpg_signature.commit
+      ).to contain_exactly(signature, another_gpg_signature)
     end
   end
   describe '#gpg_key=' do
     it 'supports the assignment of a GpgKey' do
-      gpg_signature = create(:gpg_signature, gpg_key: gpg_key)
-      expect(gpg_signature.gpg_key).to be_an_instance_of(GpgKey)
+      signature = create(:gpg_signature, gpg_key: gpg_key)
+      expect(signature.gpg_key).to be_an_instance_of(GpgKey)
     end
     it 'supports the assignment of a GpgKeySubkey' do
-      gpg_signature = create(:gpg_signature, gpg_key: gpg_key_subkey)
-      expect(gpg_signature.gpg_key).to be_an_instance_of(GpgKeySubkey)
+      signature = create(:gpg_signature, gpg_key: gpg_key_subkey)
+      expect(signature.gpg_key).to be_an_instance_of(GpgKeySubkey)
     end
     it 'clears gpg_key and gpg_key_subkey_id when passing nil' do
-      gpg_signature.update_attribute(:gpg_key, nil)
-      expect(gpg_signature.gpg_key_id).to be_nil
-      expect(gpg_signature.gpg_key_subkey_id).to be_nil
+      signature.update_attribute(:gpg_key, nil)
+      expect(signature.gpg_key_id).to be_nil
+      expect(signature.gpg_key_subkey_id).to be_nil
     end
   end
   describe '#gpg_commit' do
     context 'when commit does not exist' do
       it 'returns nil' do
-        allow(gpg_signature).to receive(:commit).and_return(nil)
-        expect(gpg_signature.gpg_commit).to be_nil
+        allow(signature).to receive(:commit).and_return(nil)
+        expect(signature.gpg_commit).to be_nil
       end
     end
     context 'when commit exists' do
       it 'returns an instance of Gitlab::Gpg::Commit' do
-        allow(gpg_signature).to receive(:commit).and_return(commit)
-        expect(gpg_signature.gpg_commit).to be_an_instance_of(Gitlab::Gpg::Commit)
+        allow(signature).to receive(:commit).and_return(commit)
+        expect(signature.gpg_commit).to be_an_instance_of(Gitlab::Gpg::Commit)
       end
     end
   end

spec/models/commit_signatures/ssh_signature_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe CommitSignatures::SshSignature do
+  # This commit is seeded from https://gitlab.com/gitlab-org/gitlab-test
+  # For instructions on how to add more seed data, see the project README
+  let(:commit_sha) { '7b5160f9bb23a3d58a0accdbe89da13b96b1ece9' }
+  let!(:project) { create(:project, :repository, path: 'sample-project') }
+  let!(:commit) { create(:commit, project: project, sha: commit_sha) }
+  let(:signature) { create(:ssh_signature, commit_sha: commit_sha) }
+  let(:ssh_key) { create(:ed25519_key_256) }
+  let(:attributes) do
+    {
+      commit_sha: commit_sha,
+      project: project,
+      key: ssh_key
+    }
+  end
+
+  it_behaves_like 'having unique enum values'
+  it_behaves_like 'commit signature'
+
+  describe 'associations' do
+    it { is_expected.to belong_to(:key).required }
+  end
+
+  describe '.by_commit_sha scope' do
+    let!(:another_signature) { create(:ssh_signature, commit_sha: '0000000000000000000000000000000000000001') }
+
+    it 'returns all signatures by sha' do
+      expect(described_class.by_commit_sha(commit_sha)).to match_array([signature])
+      expect(
+        described_class.by_commit_sha([commit_sha, another_signature.commit_sha])
+      ).to contain_exactly(signature, another_signature)
+    end
+  end
+end

spec/models/commit_signatures/x509_commit_signature_spec.rb

@@ -3,11 +3,13 @@
 require 'spec_helper'
 RSpec.describe CommitSignatures::X509CommitSignature do
+  # This commit is seeded from https://gitlab.com/gitlab-org/gitlab-test
+  # For instructions on how to add more seed data, see the project README
   let(:commit_sha) { '189a6c924013fc3fe40d6f1ec1dc20214183bc97' }
   let(:project) { create(:project, :public, :repository) }
   let!(:commit) { create(:commit, project: project, sha: commit_sha) }
   let(:x509_certificate) { create(:x509_certificate) }
-  let(:x509_signature) { create(:x509_commit_signature, commit_sha: commit_sha) }
+  let(:signature) { create(:x509_commit_signature, commit_sha: commit_sha) }
   let(:attributes) do
     {
@@ -19,38 +21,16 @@
   end
   it_behaves_like 'having unique enum values'
+  it_behaves_like 'commit signature'
   describe 'validation' do
-    it { is_expected.to validate_presence_of(:commit_sha) }
-    it { is_expected.to validate_presence_of(:project_id) }
     it { is_expected.to validate_presence_of(:x509_certificate_id) }
   end
   describe 'associations' do
-    it { is_expected.to belong_to(:project).required }
     it { is_expected.to belong_to(:x509_certificate).required }
   end
-  describe '.safe_create!' do
-    it 'finds a signature by commit sha if it existed' do
-      x509_signature
-
-      expect(described_class.safe_create!(commit_sha: commit_sha)).to eq(x509_signature)
-    end
-
-    it 'creates a new signature if it was not found' do
-      expect { described_class.safe_create!(attributes) }.to change { described_class.count }.by(1)
-    end
-
-    it 'assigns the correct attributes when creating' do
-      signature = described_class.safe_create!(attributes)
-
-      expect(signature.project).to eq(project)
-      expect(signature.commit_sha).to eq(commit_sha)
-      expect(signature.x509_certificate_id).to eq(x509_certificate.id)
-    end
-  end
-
   describe '#user' do
     context 'if email is assigned to a user' do
       let!(:user) { create(:user, email: X509Helpers::User1.certificate_email) }

spec/support/shared_examples/models/commit_signature_shared_examples.rb

+# frozen_string_literal: true
+
+RSpec.shared_examples 'commit signature' do
+  describe 'associations' do
+    it { is_expected.to belong_to(:project).required }
+  end
+
+  describe 'validation' do
+    subject { described_class.new }
+
+    it { is_expected.to validate_presence_of(:commit_sha) }
+    it { is_expected.to validate_presence_of(:project_id) }
+  end
+
+  describe '.safe_create!' do
+    it 'finds a signature by commit sha if it existed' do
+      signature
+
+      expect(described_class.safe_create!(commit_sha: commit_sha)).to eq(signature)
+    end
+
+    it 'creates a new signature if it was not found' do
+      expect { described_class.safe_create!(attributes) }.to change { described_class.count }.by(1)
+    end
+
+    it 'assigns the correct attributes when creating' do
+      signature = described_class.safe_create!(attributes)
+
+      expect(signature).to have_attributes(attributes)
+    end
+
+    it 'does not raise an error in case of a race condition' do
+      expect(described_class).to receive(:find_by).and_return(nil, instance_double(described_class, persisted?: true))
+
+      expect(described_class).to receive(:create).and_raise(ActiveRecord::RecordNotUnique)
+      allow(described_class).to receive(:create).and_call_original
+
+      described_class.safe_create!(attributes)
+    end
+  end
+
+  describe '#commit' do
+    it 'fetches the commit through the project' do
+      expect_next_instance_of(Project) do |instance|
+        expect(instance).to receive(:commit).with(commit_sha).and_return(commit)
+      end
+
+      signature.commit
+    end
+  end
+end