Geo: Verify Snippets

Leveraging the work done for Package File Verification. Most of the
changes are to the database schemas, and to specs. Verification is
behind the feature flag "geo_snippet_repository_verification".

db/migrate/20210313045617_add_verification_state_and_started_at_to_snippet_repositories.rb

+# frozen_string_literal: true
+
+class AddVerificationStateAndStartedAtToSnippetRepositories < ActiveRecord::Migration[6.0]
+  DOWNTIME = false
+
+  def change
+    change_table(:snippet_repositories) do |t|
+      t.integer :verification_state, default: 0, limit: 2, null: false
+      t.column :verification_started_at, :datetime_with_timezone
+    end
+  end
+end

db/migrate/20210313045845_add_verification_indexes_to_snippet_repositories.rb

+# frozen_string_literal: true
+
+class AddVerificationIndexesToSnippetRepositories < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  VERIFICATION_STATE_INDEX_NAME = "index_snippet_repositories_verification_state"
+  PENDING_VERIFICATION_INDEX_NAME = "index_snippet_repositories_pending_verification"
+  FAILED_VERIFICATION_INDEX_NAME = "index_snippet_repositories_failed_verification"
+  NEEDS_VERIFICATION_INDEX_NAME = "index_snippet_repositories_needs_verification"
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :snippet_repositories, :verification_state, name: VERIFICATION_STATE_INDEX_NAME
+    add_concurrent_index :snippet_repositories, :verified_at, where: "(verification_state = 0)", order: { verified_at: 'ASC NULLS FIRST' }, name: PENDING_VERIFICATION_INDEX_NAME
+    add_concurrent_index :snippet_repositories, :verification_retry_at, where: "(verification_state = 3)", order: { verification_retry_at: 'ASC NULLS FIRST' }, name: FAILED_VERIFICATION_INDEX_NAME
+    add_concurrent_index :snippet_repositories, :verification_state, where: "(verification_state = 0 OR verification_state = 3)", name: NEEDS_VERIFICATION_INDEX_NAME
+  end
+
+  def down
+    remove_concurrent_index_by_name :snippet_repositories, VERIFICATION_STATE_INDEX_NAME
+    remove_concurrent_index_by_name :snippet_repositories, PENDING_VERIFICATION_INDEX_NAME
+    remove_concurrent_index_by_name :snippet_repositories, FAILED_VERIFICATION_INDEX_NAME
+    remove_concurrent_index_by_name :snippet_repositories, NEEDS_VERIFICATION_INDEX_NAME
+  end
+end

db/schema_migrations/20210313045617

+d1e6596e9c6825e29c50523dce60fd3d0b3c067c10e210f74640ba94f7938871
\ No newline at end of file

db/schema_migrations/20210313045845

+bc6302444f7a0a858c821d971fc45a4ececd7b877020f8e920a244866c60b7a2
\ No newline at end of file

db/structure.sql

@@ -17579,6 +17579,8 @@ CREATE TABLE snippet_repositories (
     verified_at timestamp with time zone,
     verification_checksum bytea,
     verification_failure text,
+    verification_state smallint DEFAULT 0 NOT NULL,
+    verification_started_at timestamp with time zone,
     CONSTRAINT snippet_repositories_verification_failure_text_limit CHECK ((char_length(verification_failure) <= 255))
 );
 
@@ -23874,10 +23876,18 @@ CREATE INDEX index_smartcard_identities_on_user_id ON smartcard_identities USING
 
 CREATE INDEX index_snippet_on_id_and_project_id ON snippets USING btree (id, project_id);
 
+CREATE INDEX index_snippet_repositories_failed_verification ON snippet_repositories USING btree (verification_retry_at NULLS FIRST) WHERE (verification_state = 3);
+
+CREATE INDEX index_snippet_repositories_needs_verification ON snippet_repositories USING btree (verification_state) WHERE ((verification_state = 0) OR (verification_state = 3));
+
 CREATE UNIQUE INDEX index_snippet_repositories_on_disk_path ON snippet_repositories USING btree (disk_path);
 
 CREATE INDEX index_snippet_repositories_on_shard_id ON snippet_repositories USING btree (shard_id);
 
+CREATE INDEX index_snippet_repositories_pending_verification ON snippet_repositories USING btree (verified_at NULLS FIRST) WHERE (verification_state = 0);
+
+CREATE INDEX index_snippet_repositories_verification_state ON snippet_repositories USING btree (verification_state);
+
 CREATE INDEX index_snippet_repository_storage_moves_on_snippet_id ON snippet_repository_storage_moves USING btree (snippet_id);
 
 CREATE UNIQUE INDEX index_snippet_user_mentions_on_note_id ON snippet_user_mentions USING btree (note_id) WHERE (note_id IS NOT NULL);

ee/app/models/concerns/geo/repository_replicator_strategy.rb

@@ -61,5 +61,30 @@ module Geo
         full_path: model_record.repository.full_path
       )
     end
+
+    # Returns a checksum of the repository refs as defined by Gitaly
+    #
+    # @return [String] checksum of the repository refs
+    def calculate_checksum
+      repository.checksum
+    rescue Gitlab::Git::Repository::NoRepository => e
+      log_error('Repository cannot be checksummed because it does not exist', e, self.replicable_params)
+
+      raise
+    end
+
+    # Return whether it's capable of generating a checksum of itself
+    #
+    # @return [Boolean] whether it can generate a checksum
+    def checksummable?
+      repository.exists?
+    end
+
+    # Return whether it's immutable
+    #
+    # @return [Boolean] whether the replicable is immutable
+    def immutable?
+      false
+    end
   end
 end

ee/app/models/ee/snippet_repository.rb

@@ -6,6 +6,7 @@ module EE
 
     prepended do
       include ::Gitlab::Geo::ReplicableModel
+      include ::Gitlab::Geo::VerificationState
       include FromUnion
 
       with_replicator Geo::SnippetRepositoryReplicator

ee/app/models/geo/snippet_repository_registry.rb

@@ -2,6 +2,7 @@
 
 class Geo::SnippetRepositoryRegistry < Geo::BaseRegistry
   include Geo::ReplicableRegistry
+  include ::Geo::VerifiableRegistry
 
   MODEL_CLASS = ::SnippetRepository
   MODEL_FOREIGN_KEY = :snippet_repository_id

ee/app/replicators/geo/snippet_repository_replicator.rb

@@ -3,6 +3,7 @@
 module Geo
   class SnippetRepositoryReplicator < Gitlab::Geo::Replicator
     include ::Geo::RepositoryReplicatorStrategy
+    extend ::Gitlab::Utils::Override
 
     def self.model
       ::SnippetRepository
@@ -12,6 +13,11 @@ module Geo
       ::Gitlab::GitAccessSnippet
     end
 
+    override :verification_feature_flag_enabled?
+    def self.verification_feature_flag_enabled?
+      Feature.enabled?(:geo_snippet_repository_verification, default_enabled: :yaml)
+    end
+
     def repository
       model_record.repository
     end

ee/config/feature_flags/development/geo_snippet_repository_verification.yml

+---
+name: geo_snippet_repository_verification
+introduced_by_url: 
+rollout_issue_url: 
+milestone: '13.10'
+type: development
+group: group::geo
+default_enabled: false

ee/db/geo/migrate/20210313050709_add_verification_to_snippet_repository_registry.rb

+# frozen_string_literal: true
+
+class AddVerificationToSnippetRepositoryRegistry < ActiveRecord::Migration[6.0]
+  DOWNTIME = false
+
+  def change
+    add_column :snippet_repository_registry, :verification_started_at, :datetime_with_timezone
+    add_column :snippet_repository_registry, :verified_at, :datetime_with_timezone
+    add_column :snippet_repository_registry, :verification_retry_at, :datetime_with_timezone
+    add_column :snippet_repository_registry, :verification_retry_count, :integer
+    add_column :snippet_repository_registry, :verification_state, :integer, limit: 2, default: 0, null: false
+    add_column :snippet_repository_registry, :checksum_mismatch, :boolean
+    add_column :snippet_repository_registry, :verification_checksum, :binary
+    add_column :snippet_repository_registry, :verification_checksum_mismatched, :binary
+    add_column :snippet_repository_registry, :verification_failure, :string, limit: 255 # rubocop:disable Migration/PreventStrings because https://gitlab.com/gitlab-org/gitlab/-/issues/323806
+  end
+end

ee/db/geo/migrate/20210313051642_add_verification_indexes_to_snippet_repository_registry.rb

+# frozen_string_literal: true
+
+class AddVerificationIndexesToSnippetRepositoryRegistry < ActiveRecord::Migration[6.0]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+  PENDING_VERIFICATION_INDEX_NAME = "snippet_repository_registry_pending_verification"
+  FAILED_VERIFICATION_INDEX_NAME = "snippet_repository_registry_failed_verification"
+  NEEDS_VERIFICATION_INDEX_NAME = "snippet_repository_registry_needs_verification"
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :snippet_repository_registry, :verified_at, where: "(state = 2 AND verification_state = 0)", order: { verified_at: 'ASC NULLS FIRST' }, name: PENDING_VERIFICATION_INDEX_NAME
+    add_concurrent_index :snippet_repository_registry, :verification_retry_at, where: "(state = 2 AND verification_state = 3)", order: { verification_retry_at: 'ASC NULLS FIRST' }, name: FAILED_VERIFICATION_INDEX_NAME
+    add_concurrent_index :snippet_repository_registry, :verification_state, where: "(state = 2 AND (verification_state IN (0, 3)))", name: NEEDS_VERIFICATION_INDEX_NAME
+  end
+
+  def down
+    remove_concurrent_index_by_name :snippet_repository_registry, PENDING_VERIFICATION_INDEX_NAME
+    remove_concurrent_index_by_name :snippet_repository_registry, FAILED_VERIFICATION_INDEX_NAME
+    remove_concurrent_index_by_name :snippet_repository_registry, NEEDS_VERIFICATION_INDEX_NAME
+  end
+end

ee/db/geo/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 2021_02_25_200858) do
+ActiveRecord::Schema.define(version: 2021_03_13_051642) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -70,9 +70,9 @@ ActiveRecord::Schema.define(version: 2021_02_25_200858) do
     t.bigint "group_wiki_repository_id", null: false
     t.integer "state", limit: 2, default: 0, null: false
     t.integer "retry_count", limit: 2, default: 0
-    t.text "last_sync_failure"
     t.boolean "force_to_redownload"
     t.boolean "missing_on_primary"
+    t.text "last_sync_failure"
     t.index ["group_wiki_repository_id"], name: "index_g_wiki_repository_registry_on_group_wiki_repository_id", unique: true
     t.index ["retry_at"], name: "index_group_wiki_repository_registry_on_retry_at"
     t.index ["state"], name: "index_group_wiki_repository_registry_on_state"
@@ -148,6 +148,31 @@ ActiveRecord::Schema.define(version: 2021_02_25_200858) do
     t.index ["verified_at"], name: "package_file_registry_pending_verification", order: "NULLS FIRST", where: "((state = 2) AND (verification_state = 0))"
   end
 
+  create_table "pipeline_artifact_registry", force: :cascade do |t|
+    t.bigint "pipeline_artifact_id", null: false
+    t.datetime_with_timezone "created_at", null: false
+    t.datetime_with_timezone "last_synced_at"
+    t.datetime_with_timezone "retry_at"
+    t.datetime_with_timezone "verified_at"
+    t.datetime_with_timezone "verification_started_at"
+    t.datetime_with_timezone "verification_retry_at"
+    t.integer "state", limit: 2, default: 0, null: false
+    t.integer "verification_state", limit: 2, default: 0, null: false
+    t.integer "retry_count", limit: 2, default: 0
+    t.integer "verification_retry_count", limit: 2, default: 0
+    t.boolean "checksum_mismatch"
+    t.binary "verification_checksum"
+    t.binary "verification_checksum_mismatched"
+    t.string "verification_failure", limit: 255
+    t.string "last_sync_failure", limit: 255
+    t.index ["pipeline_artifact_id"], name: "index_pipeline_artifact_registry_on_pipeline_artifact_id", unique: true
+    t.index ["retry_at"], name: "index_pipeline_artifact_registry_on_retry_at"
+    t.index ["state"], name: "index_pipeline_artifact_registry_on_state"
+    t.index ["verification_retry_at"], name: "pipeline_artifact_registry_failed_verification", order: "NULLS FIRST", where: "((state = 2) AND (verification_state = 3))"
+    t.index ["verification_state"], name: "pipeline_artifact_registry_needs_verification", where: "((state = 2) AND (verification_state = ANY (ARRAY[0, 3])))"
+    t.index ["verified_at"], name: "pipeline_artifact_registry_pending_verification", order: "NULLS FIRST", where: "((state = 2) AND (verification_state = 0))"
+  end
+
   create_table "project_registry", id: :serial, force: :cascade do |t|
     t.integer "project_id", null: false
     t.datetime "last_repository_synced_at"
@@ -221,6 +246,15 @@ ActiveRecord::Schema.define(version: 2021_02_25_200858) do
     t.text "last_sync_failure"
     t.boolean "force_to_redownload"
     t.boolean "missing_on_primary"
+    t.datetime_with_timezone "verification_started_at"
+    t.datetime_with_timezone "verified_at"
+    t.datetime_with_timezone "verification_retry_at"
+    t.integer "verification_retry_count"
+    t.integer "verification_state", limit: 2, default: 0, null: false
+    t.boolean "checksum_mismatch"
+    t.binary "verification_checksum"
+    t.binary "verification_checksum_mismatched"
+    t.string "verification_failure", limit: 255
     t.index ["retry_at"], name: "index_snippet_repository_registry_on_retry_at"
     t.index ["snippet_repository_id"], name: "index_snippet_repository_registry_on_snippet_repository_id", unique: true
     t.index ["state"], name: "index_snippet_repository_registry_on_state"

ee/lib/gitlab/geo/replicator.rb

@@ -278,6 +278,8 @@ module Gitlab
         return unless self.class.enabled?
 
         publish(:updated, **updated_params)
+
+        after_verifiable_update
       end
 
       def created_params

ee/spec/lib/gitlab/geo/replicable_model_spec.rb

@@ -31,6 +31,7 @@ RSpec.describe Gitlab::Geo::ReplicableModel do
 
   it_behaves_like 'a replicable model' do
     let(:model_record) { subject }
+    let(:replicator_class) { Geo::DummyReplicator }
   end
 
   describe '#replicator' do

ee/spec/replicators/geo/snippet_repository_replicator_spec.rb

@@ -3,7 +3,9 @@
 require 'spec_helper'
 
 RSpec.describe Geo::SnippetRepositoryReplicator do
-  let(:model_record) { build(:snippet_repository, snippet: create(:snippet)) }
+  let(:snippet) { create(:snippet, :repository) }
+  let(:model_record) { snippet.snippet_repository }
 
   include_examples 'a repository replicator'
+  it_behaves_like 'a verifiable replicator'
 end

ee/spec/support/shared_examples/models/concerns/blob_replicator_strategy_shared_examples.rb

@@ -3,8 +3,11 @@
 # Include these shared examples in specs of Replicators that include
 # BlobReplicatorStrategy.
 #
-# A let variable called model_record should be defined in the spec. It should be
-# a valid, unpersisted instance of the model class.
+# Required let variables:
+#
+# - model_record: A valid, unpersisted instance of the model class. Or a valid,
+#                 persisted instance of the model class in a not-yet loaded let
+#                 variable (so we can trigger creation).
 #
 RSpec.shared_examples 'a blob replicator' do
   include EE::GeoHelpers
@@ -21,7 +24,9 @@ RSpec.shared_examples 'a blob replicator' do
   it_behaves_like 'a replicator'
 
   # This could be included in each model's spec, but including it here is DRYer.
-  include_examples 'a replicable model'
+  include_examples 'a replicable model' do
+    let(:replicator_class) { described_class }
+  end
 
   describe '#handle_after_create_commit' do
     it 'creates a Geo::Event' do

ee/spec/support/shared_examples/models/concerns/replicable_model_shared_examples.rb

@@ -2,10 +2,10 @@
 
 # Required let variables:
 #
-# - model_record: A valid, unpersisted instance of the model class
-#
-# We do not use `described_class` here, so we can include this in replicator
-# strategy shared examples instead of in *every* model spec.
+# - model_record: A valid, unpersisted instance of the model class. Or a valid,
+#                 persisted instance of the model class in a not-yet loaded let
+#                 variable (so we can trigger creation).
+# - replicator_class
 #
 # Also see ee/spec/lib/gitlab/geo/replicable_model_spec.rb:
 #
@@ -23,7 +23,9 @@ RSpec.shared_examples 'a replicable model' do
   end
 
   it 'invokes replicator.handle_after_create_commit on create' do
-    expect(model_record.replicator).to receive(:handle_after_create_commit)
+    expect_next_instance_of(replicator_class) do |replicator|
+      expect(replicator).to receive(:handle_after_create_commit)
+    end
 
     model_record.save!
   end

ee/spec/support/shared_examples/models/concerns/repository_replicator_strategy_shared_examples.rb

@@ -3,8 +3,11 @@
 # Include these shared examples in specs of Replicators that include
 # BlobReplicatorStrategy.
 #
-# A let variable called model_record should be defined in the spec. It should be
-# a valid, unpersisted instance of the model class.
+# Required let variables:
+#
+# - model_record: A valid, unpersisted instance of the model class. Or a valid,
+#                 persisted instance of the model class in a not-yet loaded let
+#                 variable (so we can trigger creation).
 #
 RSpec.shared_examples 'a repository replicator' do
   include EE::GeoHelpers
@@ -21,7 +24,9 @@ RSpec.shared_examples 'a repository replicator' do
   it_behaves_like 'a replicator'
 
   # This could be included in each model's spec, but including it here is DRYer.
-  include_examples 'a replicable model'
+  include_examples 'a replicable model' do
+    let(:replicator_class) { described_class }
+  end
 
   describe '#handle_after_update' do
     it 'creates a Geo::Event' do

ee/spec/support/shared_examples/models/concerns/verifiable_replicator_shared_examples.rb

@@ -563,6 +563,9 @@ RSpec.shared_examples 'a verifiable replicator' do
 
     context 'on a secondary' do
       before do
+        # Set the primary checksum
+        replicator.verify
+
         stub_secondary_node
       end
 

spec/lib/gitlab/background_migration/backfill_snippet_repositories_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Gitlab::BackgroundMigration::BackfillSnippetRepositories, :migration, schema: 2020_04_20_094444 do
+RSpec.describe Gitlab::BackgroundMigration::BackfillSnippetRepositories, :migration, schema: 2021_03_13_045845 do
   let(:gitlab_shell) { Gitlab::Shell.new }
   let(:users) { table(:users) }
   let(:snippets) { table(:snippets) }