Add latest changes from gitlab-org/gitlab@master

app/models/commit_signatures/ssh_signature.rb

@@ -6,13 +6,18 @@ module CommitSignatures
     include SignatureType
 
     belongs_to :key, optional: true
+    belongs_to :user, optional: true
 
     def type
       :ssh
     end
 
     def signed_by_user
-      key&.user
+      user || key&.user
+    end
+
+    def key_fingerprint_sha256
+      super || key&.fingerprint_sha256
     end
   end
 end

app/models/concerns/safely_change_column_default.rb

@@ -0,0 +1,46 @@
+# frozen_string_literal: true
+
+# == SafelyChangeColumnDefault concern.
+#
+# Contains functionality that allows safely changing a column default without downtime.
+# Without this concern, Rails can mutate the old default value to the new default value if the old default is explicitly
+# specified.
+#
+# Usage:
+#
+#   class SomeModel < ApplicationRecord
+#     include SafelyChangeColumnDefault
+#
+#     columns_changing_default :value
+#   end
+#
+#   # Assume a default of 100 for value
+#   SomeModel.create!(value: 100) # INSERT INTO some_model (value) VALUES (100)
+#   change_column_default('some_model', 'value', from: 100, to: 101)
+#   SomeModel.create!(value: 100) # INSERT INTO some_model (value) VALUES (100)
+#   # Without this concern, would be INSERT INTO some_model (value) DEFAULT VALUES and would insert 101.
+module SafelyChangeColumnDefault
+  extend ActiveSupport::Concern
+
+  class_methods do
+    # Indicate that one or more columns will have their database default change.
+    #
+    # By indicating those columns here, this helper prevents a case where explicitly writing the old database default
+    # will be mutated to the new database default.
+    def columns_changing_default(*columns)
+      self.columns_with_changing_default = columns.map(&:to_s)
+    end
+  end
+
+  included do
+    class_attribute :columns_with_changing_default, default: []
+
+    before_create do
+      columns_with_changing_default.to_a.each do |attr_name|
+        attr = @attributes[attr_name]
+
+        attribute_will_change!(attr_name) if !attr.changed? && attr.came_from_user?
+      end
+    end
+  end
+end

app/views/projects/commit/_signature_badge.html.haml

@@ -22,7 +22,7 @@
     = link_to(_('Learn more about X.509 signed commits'), help_page_path('user/project/repository/x509_signed_commits/index.md'), class: 'gl-link gl-display-block')
   - elsif signature.ssh?
     = _('SSH key fingerprint:')
-    %span.gl-font-monospace= signature.key&.fingerprint_sha256 || _('Unknown')
+    %span.gl-font-monospace= signature.key_fingerprint_sha256 || _('Unknown')
 
     = link_to(_('Learn about signing commits with SSH keys.'), help_page_path('user/project/repository/ssh_signed_commits/index.md'), class: 'gl-link gl-display-block gl-mt-3')
   - else

config/README.md

@@ -79,6 +79,7 @@ An example configuration file for Redis is in this directory under the name
 | `trace_chunks`     | `shared_state`    | [CI trace chunks](https://docs.gitlab.com/ee/administration/job_logs.html#incremental-logging-architecture) |
 | `rate_limiting`    | `cache`           | [Rate limiting](https://docs.gitlab.com/ee/user/admin_area/settings/user_and_ip_rate_limits.html) state     |
 | `sessions`         | `shared_state`    | [Sessions](https://docs.gitlab.com/ee/development/session.html#redis)                                       |
+| `repository_cache` | `cache`           | Repository related information                                                                              |
 
 If no configuration is found, or no URL is found in the configuration
 file, the default URL used is:

config/feature_flags/development/use_primary_and_secondary_stores_for_repository_cache.yml

@@ -0,0 +1,8 @@
+---
+name: use_primary_and_secondary_stores_for_repository_cache
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/107232#note_1216317991
+rollout_issue_url:
+milestone: '15.7'
+type: development
+group: group::scalability
+default_enabled: false

config/feature_flags/development/use_primary_store_as_default_for_repository_cache.yml

@@ -0,0 +1,8 @@
+---
+name: use_primary_store_as_default_for_repository_cache
+introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/107232#note_1216317991
+rollout_issue_url:
+milestone: '15.7'
+type: development
+group: group::scalability
+default_enabled: false

db/migrate/20230106125945_add_user_to_ssh_signatures.rb

@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+class AddUserToSshSignatures < Gitlab::Database::Migration[2.1]
+  def up
+    add_column :ssh_signatures, :user_id, :bigint, if_not_exists: true, null: true
+  end
+
+  def down
+    remove_column :ssh_signatures, :user_id, if_exists: true
+  end
+end

db/migrate/20230106131659_add_fingerprint_to_ssh_signatures.rb

@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+
+class AddFingerprintToSshSignatures < Gitlab::Database::Migration[2.1]
+  disable_ddl_transaction!
+
+  def up
+    add_column :ssh_signatures, :key_fingerprint_sha256, :bytea, if_not_exists: true
+  end
+
+  def down
+    remove_column :ssh_signatures, :key_fingerprint_sha256, :bytea, if_exists: true
+  end
+end

db/migrate/20230112014822_add_user_index_and_fk_to_ssh_signatures.rb

@@ -0,0 +1,20 @@
+# frozen_string_literal: true
+
+class AddUserIndexAndFkToSshSignatures < Gitlab::Database::Migration[2.1]
+  disable_ddl_transaction!
+
+  INDEX_NAME = 'index_ssh_signatures_on_user_id'
+
+  def up
+    add_concurrent_index :ssh_signatures, :user_id, name: INDEX_NAME
+    add_concurrent_foreign_key :ssh_signatures, :users, column: :user_id, on_delete: :nullify
+  end
+
+  def down
+    with_lock_retries do
+      remove_foreign_key_if_exists :ssh_signatures, column: :user_id
+    end
+
+    remove_concurrent_index_by_name :ssh_signatures, INDEX_NAME
+  end
+end

db/post_migrate/20230106124332_change_keys_relation_to_ssh_signatures.rb

@@ -0,0 +1,41 @@
+# frozen_string_literal: true
+
+class ChangeKeysRelationToSshSignatures < Gitlab::Database::Migration[2.1]
+  disable_ddl_transaction!
+
+  TARGET_COLUMN = :key_id
+
+  def up
+    add_concurrent_foreign_key(
+      :ssh_signatures,
+      :keys,
+      column: :key_id,
+      name: fk_name("#{TARGET_COLUMN}_nullify"),
+      on_delete: :nullify
+    )
+
+    with_lock_retries do
+      remove_foreign_key_if_exists(:ssh_signatures, column: TARGET_COLUMN, name: fk_name(TARGET_COLUMN))
+    end
+  end
+
+  def down
+    add_concurrent_foreign_key(
+      :ssh_signatures,
+      :keys,
+      column: :key_id,
+      name: fk_name(TARGET_COLUMN),
+      on_delete: :cascade
+    )
+
+    with_lock_retries do
+      remove_foreign_key_if_exists(:ssh_signatures, column: TARGET_COLUMN, name: fk_name("#{TARGET_COLUMN}_nullify"))
+    end
+  end
+
+  private
+
+  def fk_name(column_name)
+    concurrent_foreign_key_name(:ssh_signatures, column_name)
+  end
+end

db/schema_migrations/20230106124332

@@ -0,0 +1 @@
+7cd938dc6063a51abca80760b6c17f33e64fc73012c56ebbb8ffe4a18defa961

db/schema_migrations/20230106125945

@@ -0,0 +1 @@
+6b100c6dca62cbb73103b1e82e78d499eaa9a32b2a04109e5e8c79c5ec5b7927

db/schema_migrations/20230106131659

@@ -0,0 +1 @@
+1d111bb8f2eee2fa06070a383170ac0e8c0bfb7135d0b0d4e77bd98fc8458960

db/schema_migrations/20230112014822

@@ -0,0 +1 @@
+2501bf572453b7d77759dfd0677e9f0a0ae35c6095a3df6fa841a4b698602186

db/structure.sql

@@ -21978,7 +21978,9 @@ CREATE TABLE ssh_signatures (
     project_id bigint NOT NULL,
     key_id bigint,
     verification_status smallint DEFAULT 0 NOT NULL,
-    commit_sha bytea NOT NULL
+    commit_sha bytea NOT NULL,
+    user_id bigint,
+    key_fingerprint_sha256 bytea
 );
 
 CREATE SEQUENCE ssh_signatures_id_seq
@@ -31161,6 +31163,8 @@ CREATE INDEX index_ssh_signatures_on_key_id ON ssh_signatures USING btree (key_i
 
 CREATE INDEX index_ssh_signatures_on_project_id ON ssh_signatures USING btree (project_id);
 
+CREATE INDEX index_ssh_signatures_on_user_id ON ssh_signatures USING btree (user_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);
@@ -33190,6 +33194,9 @@ ALTER TABLE ONLY dast_sites
 ALTER TABLE ONLY issue_customer_relations_contacts
     ADD CONSTRAINT fk_0c0037f723 FOREIGN KEY (issue_id) REFERENCES issues(id) ON DELETE CASCADE;
 
+ALTER TABLE ONLY ssh_signatures
+    ADD CONSTRAINT fk_0c83baaa5f FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE SET NULL;
+
 ALTER TABLE ONLY web_hooks
     ADD CONSTRAINT fk_0c8ca6d9d1 FOREIGN KEY (project_id) REFERENCES projects(id) ON DELETE CASCADE;
 
@@ -33736,6 +33743,9 @@ ALTER TABLE ONLY lfs_objects_projects
 ALTER TABLE ONLY merge_requests
     ADD CONSTRAINT fk_a6963e8447 FOREIGN KEY (target_project_id) REFERENCES projects(id) ON DELETE CASCADE;
 
+ALTER TABLE ONLY ssh_signatures
+    ADD CONSTRAINT fk_aa1efbe865 FOREIGN KEY (key_id) REFERENCES keys(id) ON DELETE SET NULL;
+
 ALTER TABLE ONLY epics
     ADD CONSTRAINT fk_aa5798e761 FOREIGN KEY (closed_by_id) REFERENCES users(id) ON DELETE SET NULL;
 
@@ -34054,9 +34064,6 @@ 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;
 

doc/administration/geo/replication/configuration.md

@@ -104,7 +104,7 @@ keys must be manually replicated to the **secondary** site.
 1. Make a backup of any existing SSH host keys:
 
    ```shell
-   find /etc/ssh -iname ssh_host_* -exec cp {} {}.backup.`date +%F` \;
+   find /etc/ssh -iname 'ssh_host_*' -exec cp {} {}.backup.`date +%F` \;
    ```
 
 1. Copy OpenSSH host keys from the **primary** site:

doc/architecture/blueprints/ci_pipeline_components/index.md

@@ -148,9 +148,13 @@ A component should not produce side effects by being included and should be
 
 Making components predictable is a process, and we may not be able to achieve
 this without significantly redesigning CI templates, what could be disruptive
-for users and customers right now. The predictability, determinism, referential
-transparency and making CI components predictable is still important for us,
-but we may be unable to achieve it early iterations.
+for users and customers right now. We initially considered restricting some
+top-level keywords, like `include: remote:` to make components more
+deterministic, but eventually agreed that we first need to iterate on the MVP
+to better understand the design that is required to make components more
+predictable. The predictability, determinism, referential transparency and
+making CI components predictable is still important for us, but we may be
+unable to achieve it early iterations.
 
 ## Structure of a component
 

doc/development/database/avoiding_downtime_in_migrations.md

@@ -282,6 +282,62 @@ Example migration:
   end
 ```
 
+## Changing column defaults
+
+Changing column defaults is difficult because of how Rails handles values
+that are equal to the default.
+
+If running code ever explicitly writes the old default value of a column, you must follow a multi-step
+process to prevent Rails replacing the old default with the new default in INSERT queries that explicitly
+specify the old default.
+
+Doing this requires steps in two minor releases:
+
+1. Add the `SafelyChangeColumnDefault` concern to the model and change the default in a post-migration.
+1. Clean up the `SafelyChangeColumnDefault` concern in the next minor release.
+
+We must wait a minor release before cleaning up the `SafelyChangeColumnDefault` because self-managed
+releases bundle an entire minor release into a single zero-downtime deployment.
+
+### Step 1: Add the `SafelyChangeColumnDefault` concern to the model and change the default in a post-migration
+
+The first step is to mark the column as safe to change in application code.
+
+```ruby
+class Ci::Build < ApplicationRecord
+  include SafelyChangeColumnDefault
+
+  columns_changing_default :partition_id
+end
+```
+
+Then create a **post-deployment migration** to change the default:
+
+```shell
+bundle exec rails g post_deployment_migration change_ci_builds_default
+```
+
+```ruby
+class ChangeCiBuildsDefault < Gitlab::Database::Migration[2.1]
+  def up
+    change_column_default('ci_builds', 'partition_id', from: 100, to: 101)
+  end
+
+  def down
+    change_column_default('ci_builds', 'partition_id', from: 101, to: 100)
+  end
+end
+```
+
+You can consider [enabling lock retries](../migration_style_guide.md#usage-with-transactional-migrations)
+when you run a migration on big tables, because it might take some time to
+acquire a lock on this table.
+
+### Step 2: Clean up the `SafelyChangeColumnDefault` concern in the next minor release
+
+In the next minor release, create a new merge request to remove the `columns_changing_default` call. Also remove the `SafelyChangeColumnDefault` include
+if it is not needed for a different column.
+
 ## Changing The Schema For Large Tables
 
 While `change_column_type_concurrently` and `rename_column_concurrently` can be

doc/development/migration_style_guide.md

@@ -427,6 +427,9 @@ end
 
 #### Changing default value for a column
 
+Note that changing column defaults can cause application downtime if a multi-release process is not followed.
+See [avoiding downtime in migrations for changing column defaults](database/avoiding_downtime_in_migrations.md#changing-column-defaults) for details.
+
 ```ruby
 enable_lock_retries!
 

doc/user/group/saml_sso/group_sync.md

@@ -35,15 +35,32 @@ WARNING:
 To prevent users being accidentally removed from the GitLab group, follow these instructions closely before
 enabling Group Sync in GitLab.
 
-To configure SAML Group Sync:
+To configure SAML Group Sync for self-managed GitLab instances:
 
-1. Configure the identity Provider:
-   - For self-managed GitLab, see the [SAML OmniAuth Provider documentation](../../../integration/saml.md).
-   - For GitLab.com, see the [SAML SSO for GitLab.com groups documentation](index.md).
+1. Configure the [SAML OmniAuth Provider](../../../integration/saml.md).
+1. Ensure your SAML identity provider sends an attribute statement with the same name as the value of the `groups_attribute` setting. See the following attribute statement example for reference:
 
-1. Capture [a SAML response](troubleshooting.md#saml-debugging-tools) during the sign-in process to confirm your SAML identity provider sends an attribute statement:
-   - For self-managed GitLab, with the same name as the value of the `groups_attribute` setting.
-   - For GitLab.com, named `Groups` or `groups`.
+   ```ruby
+   gitlab_rails['omniauth_providers'] = [
+     {
+       name: "saml",
+       label: "Provider name", # optional label for login button, defaults to "Saml",
+       groups_attribute: 'Groups',
+       args: {
+         assertion_consumer_service_url: "https://gitlab.example.com/users/auth/saml/callback",
+         idp_cert_fingerprint: "43:51:43:a1:b5:fc:8b:b7:0a:3a:a9:b1:0f:66:73:a8",
+         idp_sso_target_url: "https://login.example.com/idp",
+         issuer: "https://gitlab.example.com",
+         name_identifier_format: "urn:oasis:names:tc:SAML:2.0:nameid-format:persistent"
+       }
+     }
+   ]
+   ```
+
+To configure SAML Group Sync for GitLab.com instances:
+
+1. See [SAML SSO for GitLab.com groups](index.md).
+1. Ensure your SAML identity provider sends an attribute statement named `Groups` or `groups`.
 
 NOTE:
 The value for `Groups` or `groups` in the SAML response may be either the group name or an ID.

lib/gitlab/redis/multi_store.rb

@@ -217,6 +217,17 @@ module Gitlab
           extra.merge(command_name: command_name, instance_name: instance_name))
       end
 
+      def ping(message = nil)
+        if use_primary_and_secondary_stores?
+          # Both stores have to response success for the ping to be considered success.
+          # We assume both stores cannot return different responses (only both "PONG" or both echo the message).
+          # If either store is not reachable, an Error will be raised anyway thus taking any response works.
+          [primary_store, secondary_store].map { |store| store.ping(message) }.first
+        else
+          default_store.ping(message)
+        end
+      end
+
       private
 
       # @return [Boolean]

lib/gitlab/redis/repository_cache.rb

@@ -3,9 +3,30 @@
 module Gitlab
   module Redis
     class RepositoryCache < ::Gitlab::Redis::Wrapper
-      # The data we store on RepositoryCache used to be stored on Cache.
-      def self.config_fallback
-        Cache
+      class << self
+        # The data we store on RepositoryCache used to be stored on Cache.
+        def config_fallback
+          Cache
+        end
+
+        def cache_store
+          @cache_store ||= ActiveSupport::Cache::RedisCacheStore.new(
+            redis: pool,
+            compress: Gitlab::Utils.to_boolean(ENV.fetch('ENABLE_REDIS_CACHE_COMPRESSION', '1')),
+            namespace: Cache::CACHE_NAMESPACE,
+            # Cache should not grow forever
+            expires_in: ENV.fetch('GITLAB_RAILS_CACHE_DEFAULT_TTL_SECONDS', 8.hours).to_i
+          )
+        end
+
+        private
+
+        def redis
+          primary_store = ::Redis.new(params)
+          secondary_store = ::Redis.new(config_fallback.params)
+
+          MultiStore.new(primary_store, secondary_store, store_name)
+        end
       end
     end
   end

lib/gitlab/repository_cache.rb

@@ -5,7 +5,7 @@ module Gitlab
   class RepositoryCache
     attr_reader :repository, :namespace, :backend
 
-    def initialize(repository, extra_namespace: nil, backend: Rails.cache)
+    def initialize(repository, extra_namespace: nil, backend: self.class.store)
       @repository = repository
       @namespace = "#{repository.full_path}"
       @namespace += ":#{repository.project.id}" if repository.project
@@ -48,5 +48,14 @@ module Gitlab
 
       value
     end
+
+    def self.store
+      if Feature.enabled?(:use_primary_and_secondary_stores_for_repository_cache) ||
+          Feature.enabled?(:use_primary_store_as_default_for_repository_cache)
+        Gitlab::Redis::RepositoryCache.cache_store
+      else
+        Rails.cache
+      end
+    end
   end
 end

lib/gitlab/repository_hash_cache.rb

@@ -139,8 +139,17 @@ module Gitlab
 
     private
 
+    def cache
+      if Feature.enabled?(:use_primary_and_secondary_stores_for_repository_cache) ||
+          Feature.enabled?(:use_primary_store_as_default_for_repository_cache)
+        Gitlab::Redis::RepositoryCache
+      else
+        Gitlab::Redis::Cache
+      end
+    end
+
     def with(&blk)
-      Gitlab::Redis::Cache.with(&blk) # rubocop:disable CodeReuse/ActiveRecord
+      cache.with(&blk) # rubocop:disable CodeReuse/ActiveRecord
     end
 
     # Take a hash and convert both keys and values to strings, for insertion into Redis.

lib/gitlab/repository_set_cache.rb

@@ -64,5 +64,20 @@ module Gitlab
         redis.sscan_each(full_key, match: pattern)
       end
     end
+
+    private
+
+    def cache
+      if Feature.enabled?(:use_primary_and_secondary_stores_for_repository_cache) ||
+          Feature.enabled?(:use_primary_store_as_default_for_repository_cache)
+        Gitlab::Redis::RepositoryCache
+      else
+        Gitlab::Redis::Cache
+      end
+    end
+
+    def with(&blk)
+      cache.with(&blk) # rubocop:disable CodeReuse/ActiveRecord
+    end
   end
 end

lib/gitlab/ssh/commit.rb

@@ -16,6 +16,8 @@ module Gitlab
           commit_sha: @commit.sha,
           project: @commit.project,
           key_id: signature.signed_by_key&.id,
+          key_fingerprint_sha256: signature.key_fingerprint,
+          user_id: signature.signed_by_key&.user_id,
           verification_status: signature.verification_status
         }
       end

lib/gitlab/ssh/signature.rb

@@ -41,6 +41,10 @@ module Gitlab
         end
       end
 
+      def key_fingerprint
+        strong_memoize(:key_fingerprint) { signature&.public_key&.fingerprint }
+      end
+
       private
 
       def all_attributes_present?
@@ -77,10 +81,6 @@ module Gitlab
           nil
         end
       end
-
-      def key_fingerprint
-        strong_memoize(:key_fingerprint) { signature&.public_key&.fingerprint }
-      end
     end
   end
 end

spec/controllers/projects/pipelines_controller_spec.rb

@@ -84,6 +84,13 @@ RSpec.describe Projects::PipelinesController do
     end
 
     context 'when performing gitaly calls', :request_store do
+      before do
+        # To prevent double writes / fallback read due to MultiStore which is failing the `Gitlab::GitalyClient
+        # .get_request_count` expectation.
+        stub_feature_flags(use_primary_store_as_default_for_repository_cache: false)
+        stub_feature_flags(use_primary_and_secondary_stores_for_repository_cache: false)
+      end
+
       it 'limits the Gitaly requests' do
         # Isolate from test preparation (Repository#exists? is also cached in RequestStore)
         RequestStore.end!

spec/features/issues/user_creates_issue_spec.rb

@@ -161,6 +161,11 @@ RSpec.describe "User creates issue", feature_category: :team_planning do
       let(:project) { create(:project, :public, :repository) }
 
       before do
+        # With multistore feature flags enabled (using an actual Redis store instead of NullStore),
+        # it somehow writes an invalid content to Redis and the specs would fail.
+        stub_feature_flags(use_primary_and_secondary_stores_for_repository_cache: false)
+        stub_feature_flags(use_primary_store_as_default_for_repository_cache: false)
+
         project.repository.create_file(
           user,
           '.gitlab/issue_templates/bug.md',

spec/lib/gitlab/import_export/snippets_repo_restorer_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Gitlab::ImportExport::SnippetsRepoRestorer do
+RSpec.describe Gitlab::ImportExport::SnippetsRepoRestorer, :clean_gitlab_redis_repository_cache, feature_category: :importers do
   describe 'bundle a snippet Git repo' do
     let_it_be(:user) { create(:user) }
     let_it_be(:project) { create(:project, namespace: user.namespace) }
@@ -26,9 +26,18 @@ RSpec.describe Gitlab::ImportExport::SnippetsRepoRestorer do
     shared_examples 'imports snippet repositories' do
       before do
         snippet1.snippet_repository&.delete
+        # We need to explicitly invalidate repository.exists? from cache by calling repository.expire_exists_cache.
+        # Previously, we didn't have to do this because snippet1.repository_exists? would hit Rails.cache, which is a
+        # NullStore, thus cache.read would always be false.
+        # Now, since we are using a separate instance of Redis, ie Gitlab::Redis::RepositoryCache,
+        # snippet.repository_exists? would still be true because snippet.repository.remove doesn't invalidate the
+        # cache (snippet.repository.remove only makes gRPC call to Gitaly).
+        # See https://gitlab.com/gitlab-org/gitlab/-/merge_requests/107232#note_1214358593 for more.
+        snippet1.repository.expire_exists_cache
         snippet1.repository.remove
 
         snippet2.snippet_repository&.delete
+        snippet2.repository.expire_exists_cache
         snippet2.repository.remove
       end
 

spec/lib/gitlab/instrumentation_helper_spec.rb

@@ -4,7 +4,8 @@ require 'spec_helper'
 require 'rspec-parameterized'
 require 'support/helpers/rails_helpers'
 
-RSpec.describe Gitlab::InstrumentationHelper do
+RSpec.describe Gitlab::InstrumentationHelper, :clean_gitlab_redis_repository_cache, :clean_gitlab_redis_cache,
+               feature_category: :scalability do
   using RSpec::Parameterized::TableSyntax
 
   describe '.add_instrumentation_data', :request_store do
@@ -22,21 +23,44 @@ RSpec.describe Gitlab::InstrumentationHelper do
       expect(payload).to include(db_count: 0, db_cached_count: 0, db_write_count: 0)
     end
 
-    context 'when Gitaly calls are made' do
-      it 'adds Gitaly data and omits Redis data' do
-        project = create(:project)
-        RequestStore.clear!
-        project.repository.exists?
+    shared_examples 'make Gitaly calls' do
+      context 'when Gitaly calls are made' do
+        it 'adds Gitaly and Redis data' do
+          project = create(:project)
+          RequestStore.clear!
+          project.repository.exists?
 
-        subject
+          subject
 
-        expect(payload[:gitaly_calls]).to eq(1)
-        expect(payload[:gitaly_duration_s]).to be >= 0
-        expect(payload[:redis_calls]).to be_nil
-        expect(payload[:redis_duration_ms]).to be_nil
+          expect(payload[:gitaly_calls]).to eq(1)
+          expect(payload[:gitaly_duration_s]).to be >= 0
+          # With MultiStore, the number of `redis_calls` depends on whether primary_store
+          # (Gitlab::Redis::Repositorycache) and secondary_store (Gitlab::Redis::Cache) are of the same instance.
+          # In GitLab.com CI, primary and secondary are the same instance, thus only 1 call being made. If primary
+          # and secondary are different instances, an additional fallback read to secondary_store will be made because
+          # the first `get` call is a cache miss. Then, the following expect will fail.
+          expect(payload[:redis_calls]).to eq(1)
+          expect(payload[:redis_duration_ms]).to be_nil
+        end
       end
     end
 
+    context 'when multistore ff use_primary_and_secondary_stores_for_repository_cache is enabled' do
+      before do
+        stub_feature_flags(use_primary_and_secondary_stores_for_repository_cache: true)
+      end
+
+      it_behaves_like 'make Gitaly calls'
+    end
+
+    context 'when multistore ff use_primary_and_secondary_stores_for_repository_cache is disabled' do
+      before do
+        stub_feature_flags(use_primary_and_secondary_stores_for_repository_cache: false)
+      end
+
+      it_behaves_like 'make Gitaly calls'
+    end
+
     context 'when Redis calls are made' do
       it 'adds Redis data and omits Gitaly data' do
         stub_rails_env('staging') # to avoid raising CrossSlotError

spec/lib/gitlab/redis/multi_store_spec.rb

@@ -940,6 +940,98 @@ RSpec.describe Gitlab::Redis::MultiStore, feature_category: :redis do
     include_examples 'pipelined command', :pipelined
   end
 
+  describe '#ping' do
+    subject { multi_store.ping }
+
+    context 'when using both stores' do
+      before do
+        allow(multi_store).to receive(:use_primary_and_secondary_stores?).and_return(true)
+      end
+
+      context 'without message' do
+        it 'returns PONG' do
+          expect(subject).to eq('PONG')
+        end
+      end
+
+      context 'with message' do
+        it 'returns the same message' do
+          expect(multi_store.ping('hello world')).to eq('hello world')
+        end
+      end
+
+      shared_examples 'returns an error' do
+        before do
+          allow(store).to receive(:ping).and_raise('boom')
+        end
+
+        it 'returns the error' do
+          expect { subject }.to raise_error('boom')
+        end
+      end
+
+      context 'when primary store returns an error' do
+        let(:store) { primary_store }
+
+        it_behaves_like 'returns an error'
+      end
+
+      context 'when secondary store returns an error' do
+        let(:store) { secondary_store }
+
+        it_behaves_like 'returns an error'
+      end
+    end
+
+    shared_examples 'single store as default store' do
+      context 'when the store retuns success' do
+        it 'returns response from the respective store' do
+          expect(store).to receive(:ping).and_return('PONG')
+
+          subject
+
+          expect(subject).to eq('PONG')
+        end
+      end
+
+      context 'when the store returns an error' do
+        before do
+          allow(store).to receive(:ping).and_raise('boom')
+        end
+
+        it 'returns the error' do
+          expect { subject }.to raise_error('boom')
+        end
+      end
+    end
+
+    context 'when using only one store' do
+      before do
+        allow(multi_store).to receive(:use_primary_and_secondary_stores?).and_return(false)
+      end
+
+      context 'when using primary_store as default store' do
+        let(:store) { primary_store }
+
+        before do
+          allow(multi_store).to receive(:use_primary_store_as_default?).and_return(true)
+        end
+
+        it_behaves_like 'single store as default store'
+      end
+
+      context 'when using secondary_store as default store' do
+        let(:store) { secondary_store }
+
+        before do
+          allow(multi_store).to receive(:use_primary_store_as_default?).and_return(false)
+        end
+
+        it_behaves_like 'single store as default store'
+      end
+    end
+  end
+
   context 'with unsupported command' do
     let(:counter) { Gitlab::Metrics::NullMetric.instance }
 

spec/lib/gitlab/redis/repository_cache_spec.rb

@@ -4,4 +4,54 @@ require 'spec_helper'
 
 RSpec.describe Gitlab::Redis::RepositoryCache, feature_category: :scalability do
   include_examples "redis_new_instance_shared_examples", 'repository_cache', Gitlab::Redis::Cache
+  include_examples "redis_shared_examples"
+
+  describe '#pool' do
+    let(:config_new_format_host) { "spec/fixtures/config/redis_new_format_host.yml" }
+    let(:config_new_format_socket) { "spec/fixtures/config/redis_new_format_socket.yml" }
+
+    subject { described_class.pool }
+
+    before do
+      redis_clear_raw_config!(described_class)
+      redis_clear_raw_config!(Gitlab::Redis::Cache)
+
+      allow(described_class).to receive(:config_file_name).and_return(config_new_format_host)
+      allow(Gitlab::Redis::Cache).to receive(:config_file_name).and_return(config_new_format_socket)
+    end
+
+    after do
+      redis_clear_raw_config!(described_class)
+      redis_clear_raw_config!(Gitlab::Redis::Cache)
+    end
+
+    around do |example|
+      clear_pool
+      example.run
+    ensure
+      clear_pool
+    end
+
+    it 'instantiates an instance of MultiStore' do
+      subject.with do |redis_instance|
+        expect(redis_instance).to be_instance_of(::Gitlab::Redis::MultiStore)
+
+        expect(redis_instance.primary_store.connection[:id]).to eq("redis://test-host:6379/99")
+        expect(redis_instance.secondary_store.connection[:id]).to eq("unix:///path/to/redis.sock/0")
+
+        expect(redis_instance.instance_name).to eq('RepositoryCache')
+      end
+    end
+
+    it_behaves_like 'multi store feature flags', :use_primary_and_secondary_stores_for_repository_cache,
+                    :use_primary_store_as_default_for_repository_cache
+  end
+
+  describe '#raw_config_hash' do
+    it 'has a legacy default URL' do
+      expect(subject).to receive(:fetch_config).and_return(false)
+
+      expect(subject.send(:raw_config_hash)).to eq(url: 'redis://localhost:6380')
+    end
+  end
 end

spec/lib/gitlab/repository_cache/preloader_spec.rb

@@ -6,49 +6,75 @@ RSpec.describe Gitlab::RepositoryCache::Preloader, :use_clean_rails_redis_cachin
   let(:projects) { create_list(:project, 2, :repository) }
   let(:repositories) { projects.map(&:repository) }
 
-  describe '#preload' do
-    context 'when the values are already cached' do
-      before do
-        # Warm the cache but use a different model so they are not memoized
-        repos = Project.id_in(projects).order(:id).map(&:repository)
+  before do
+    stub_feature_flags(use_primary_store_as_default_for_repository_cache: false)
+  end
 
-        allow(repos[0].head_tree).to receive(:readme_path).and_return('README.txt')
-        allow(repos[1].head_tree).to receive(:readme_path).and_return('README.md')
+  shared_examples 'preload' do
+    describe '#preload' do
+      context 'when the values are already cached' do
+        before do
+          # Warm the cache but use a different model so they are not memoized
+          repos = Project.id_in(projects).order(:id).map(&:repository)
 
-        repos.map(&:exists?)
-        repos.map(&:readme_path)
+          allow(repos[0].head_tree).to receive(:readme_path).and_return('README.txt')
+          allow(repos[1].head_tree).to receive(:readme_path).and_return('README.md')
+
+          repos.map(&:exists?)
+          repos.map(&:readme_path)
+        end
+
+        it 'prevents individual cache reads for cached methods' do
+          expect(cache).to receive(:read_multi).once.and_call_original
+
+          described_class.new(repositories).preload(
+            %i[exists? readme_path]
+          )
+
+          expect(cache).not_to receive(:read)
+          expect(cache).not_to receive(:write)
+
+          expect(repositories[0].exists?).to eq(true)
+          expect(repositories[0].readme_path).to eq('README.txt')
+
+          expect(repositories[1].exists?).to eq(true)
+          expect(repositories[1].readme_path).to eq('README.md')
+        end
       end
 
-      it 'prevents individual cache reads for cached methods' do
-        expect(Rails.cache).to receive(:read_multi).once.and_call_original
+      context 'when values are not cached' do
+        it 'reads and writes from cache individually' do
+          described_class.new(repositories).preload(
+            %i[exists? has_visible_content?]
+          )
 
-        described_class.new(repositories).preload(
-          %i[exists? readme_path]
-        )
+          expect(cache).to receive(:read).exactly(4).times
+          expect(cache).to receive(:write).exactly(4).times
 
-        expect(Rails.cache).not_to receive(:read)
-        expect(Rails.cache).not_to receive(:write)
-
-        expect(repositories[0].exists?).to eq(true)
-        expect(repositories[0].readme_path).to eq('README.txt')
-
-        expect(repositories[1].exists?).to eq(true)
-        expect(repositories[1].readme_path).to eq('README.md')
-      end
-    end
-
-    context 'when values are not cached' do
-      it 'reads and writes from cache individually' do
-        described_class.new(repositories).preload(
-          %i[exists? has_visible_content?]
-        )
-
-        expect(Rails.cache).to receive(:read).exactly(4).times
-        expect(Rails.cache).to receive(:write).exactly(4).times
-
-        repositories.each(&:exists?)
-        repositories.each(&:has_visible_content?)
+          repositories.each(&:exists?)
+          repositories.each(&:has_visible_content?)
+        end
       end
     end
   end
+
+  context 'when use_primary_and_secondary_stores_for_repository_cache feature flag is enabled' do
+    let(:cache) { Gitlab::RepositoryCache.store }
+
+    before do
+      stub_feature_flags(use_primary_and_secondary_stores_for_repository_cache: true)
+    end
+
+    it_behaves_like 'preload'
+  end
+
+  context 'when use_primary_and_secondary_stores_for_repository_cache feature flag is disabled' do
+    let(:cache) { Rails.cache }
+
+    before do
+      stub_feature_flags(use_primary_and_secondary_stores_for_repository_cache: false)
+    end
+
+    it_behaves_like 'preload'
+  end
 end

spec/lib/gitlab/repository_hash_cache_spec.rb

@@ -69,20 +69,35 @@ RSpec.describe Gitlab::RepositoryHashCache, :clean_gitlab_redis_cache do
     end
   end
 
-  describe "#key?" do
-    subject { cache.key?(:example, "test") }
+  shared_examples "key?" do
+    describe "#key?" do
+      subject { cache.key?(:example, "test") }
 
-    context "key exists" do
-      before do
-        cache.write(:example, test_hash)
+      context "key exists" do
+        before do
+          cache.write(:example, test_hash)
+        end
+
+        it { is_expected.to be(true) }
       end
 
-      it { is_expected.to be(true) }
+      context "key doesn't exist" do
+        it { is_expected.to be(false) }
+      end
+    end
+  end
+
+  context "when both multistore FF is enabled" do
+    it_behaves_like "key?"
+  end
+
+  context "when both multistore FF is disabled" do
+    before do
+      stub_feature_flags(use_primary_and_secondary_stores_for_repository_cache: false)
+      stub_feature_flags(use_primary_store_as_default_for_repository_cache: false)
     end
 
-    context "key doesn't exist" do
-      it { is_expected.to be(false) }
-    end
+    it_behaves_like "key?"
   end
 
   describe "#read_members" do

spec/lib/gitlab/ssh/commit_spec.rb

@@ -1,9 +1,10 @@
 # frozen_string_literal: true
 require 'spec_helper'
 
-RSpec.describe Gitlab::Ssh::Commit do
+RSpec.describe Gitlab::Ssh::Commit, feature_category: :source_code_management do
   let_it_be(:project) { create(:project, :repository) }
   let_it_be(:signed_by_key) { create(:key) }
+  let_it_be(:fingerprint) { signed_by_key.fingerprint_sha256 }
 
   let(:commit) { create(:commit, project: project) }
   let(:signature_text) { 'signature_text' }
@@ -19,8 +20,11 @@ RSpec.describe Gitlab::Ssh::Commit do
       .with(Gitlab::Git::Repository, commit.sha)
       .and_return(signature_data)
 
-    allow(verifier).to receive(:verification_status).and_return(verification_status)
-    allow(verifier).to receive(:signed_by_key).and_return(signed_by_key)
+    allow(verifier).to receive_messages({
+      verification_status: verification_status,
+      signed_by_key: signed_by_key,
+      key_fingerprint: fingerprint
+    })
 
     allow(Gitlab::Ssh::Signature).to receive(:new)
       .with(signature_text, signed_text, commit.committer_email)
@@ -44,6 +48,8 @@ RSpec.describe Gitlab::Ssh::Commit do
           commit_sha: commit.sha,
           project: project,
           key_id: signed_by_key.id,
+          key_fingerprint_sha256: signed_by_key.fingerprint_sha256,
+          user_id: signed_by_key.user_id,
           verification_status: 'verified'
         )
       end
@@ -51,6 +57,7 @@ RSpec.describe Gitlab::Ssh::Commit do
 
     context 'when signed_by_key is nil' do
       let_it_be(:signed_by_key) { nil }
+      let_it_be(:fingerprint) { nil }
 
       let(:verification_status) { :unknown_key }
 
@@ -59,6 +66,8 @@ RSpec.describe Gitlab::Ssh::Commit do
           commit_sha: commit.sha,
           project: project,
           key_id: nil,
+          key_fingerprint_sha256: nil,
+          user_id: nil,
           verification_status: 'unknown_key'
         )
       end

spec/lib/gitlab/ssh/signature_spec.rb

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Gitlab::Ssh::Signature do
+RSpec.describe Gitlab::Ssh::Signature, feature_category: :source_code_management do
   # ssh-keygen -t ed25519
   let_it_be(:committer_email) { 'ssh-commit-test@example.com' }
   let_it_be(:public_key_text) { 'ssh-ed25519 AAAAC3NzaC1lZDI1NTE5AAAAIHZ8NHEnCIpC4mnot+BRxv6L+fq+TnN1CgsRrHWLmfwb' }
@@ -267,4 +267,10 @@ RSpec.describe Gitlab::Ssh::Signature do
       end
     end
   end
+
+  describe '#key_fingerprint' do
+    it 'returns the pubkey sha256 fingerprint' do
+      expect(signature.key_fingerprint).to eq('dw7gPSvYtkCBU+BbTolbbckUEX3sL6NsGIJTQ4PYEnM')
+    end
+  end
 end

spec/models/commit_signatures/ssh_signature_spec.rb

@@ -2,24 +2,30 @@
 
 require 'spec_helper'
 
-RSpec.describe CommitSignatures::SshSignature do
+RSpec.describe CommitSignatures::SshSignature, feature_category: :source_code_management 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_it_be(:commit_sha) { '7b5160f9bb23a3d58a0accdbe89da13b96b1ece9' }
   let_it_be(:project) { create(:project, :repository, path: 'sample-project') }
   let_it_be(:commit) { create(:commit, project: project, sha: commit_sha) }
   let_it_be(:ssh_key) { create(:ed25519_key_256) }
+  let_it_be(:user) { ssh_key.user }
+  let_it_be(:key_fingerprint) { ssh_key.fingerprint_sha256 }
+
+  let(:signature) do
+    create(:ssh_signature, commit_sha: commit_sha, key: ssh_key, key_fingerprint_sha256: key_fingerprint, user: user)
+  end
 
   let(:attributes) do
     {
       commit_sha: commit_sha,
       project: project,
-      key: ssh_key
+      key: ssh_key,
+      key_fingerprint_sha256: key_fingerprint,
+      user: user
     }
   end
 
-  let(:signature) { create(:ssh_signature, commit_sha: commit_sha, key: ssh_key) }
-
   it_behaves_like 'having unique enum values'
   it_behaves_like 'commit signature'
   it_behaves_like 'signature with type checking', :ssh
@@ -39,9 +45,31 @@ RSpec.describe CommitSignatures::SshSignature do
     end
   end
 
+  describe '#key_fingerprint_sha256' do
+    it 'returns the fingerprint_sha256 associated with the SSH key' do
+      expect(signature.key_fingerprint_sha256).to eq(key_fingerprint)
+    end
+
+    context 'when the SSH key is no longer associated with the signature' do
+      it 'returns the fingerprint_sha256 stored in signature' do
+        signature.update!(key_id: nil)
+
+        expect(signature.key_fingerprint_sha256).to eq(key_fingerprint)
+      end
+    end
+  end
+
   describe '#signed_by_user' do
     it 'returns the user associated with the SSH key' do
       expect(signature.signed_by_user).to eq(ssh_key.user)
     end
+
+    context 'when the SSH key is no longer associated with the signature' do
+      it 'returns the user stored in signature' do
+        signature.update!(key_id: nil)
+
+        expect(signature.signed_by_user).to eq(user)
+      end
+    end
   end
 end

spec/models/concerns/safely_change_column_default_spec.rb

@@ -0,0 +1,75 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe SafelyChangeColumnDefault, feature_category: :database do
+  include Gitlab::Database::DynamicModelHelpers
+  before do
+    ApplicationRecord.connection.execute(<<~SQL)
+      CREATE TABLE _test_gitlab_main_data(
+        id bigserial primary key not null,
+        value bigint default 1
+      );
+    SQL
+  end
+
+  let!(:model) do
+    define_batchable_model('_test_gitlab_main_data', connection: ApplicationRecord.connection).tap do |model|
+      model.include(described_class)
+      model.columns_changing_default(:value)
+      model.columns # Force the schema cache to populate
+    end
+  end
+
+  def alter_default(new_default)
+    ApplicationRecord.connection.execute(<<~SQL)
+      ALTER TABLE _test_gitlab_main_data ALTER COLUMN value SET DEFAULT #{new_default}
+    SQL
+  end
+
+  def recorded_insert_queries(&block)
+    recorder = ActiveRecord::QueryRecorder.new
+    recorder.record(&block)
+
+    recorder.log.select { |q| q.include?('INSERT INTO') }
+  end
+
+  def query_includes_value_column?(query)
+    parsed = PgQuery.parse(query)
+    parsed.tree.stmts.first.stmt.insert_stmt.cols.any? { |node| node.res_target.name == 'value' }
+  end
+
+  it 'forces the column to be written on a change' do
+    queries = recorded_insert_queries do
+      model.create!(value: 1)
+    end
+
+    expect(queries.length).to eq(1)
+
+    expect(query_includes_value_column?(queries.first)).to be_truthy
+  end
+
+  it 'does not write the column without a change' do
+    queries = recorded_insert_queries do
+      model.create!
+    end
+
+    expect(queries.length).to eq(1)
+    expect(query_includes_value_column?(queries.first)).to be_falsey
+  end
+
+  it 'does not send the old column value if the default has changed' do
+    alter_default(2)
+    model.create!
+
+    expect(model.pluck(:value)).to contain_exactly(2)
+  end
+
+  it 'prevents writing new default in place of the old default' do
+    alter_default(2)
+
+    model.create!(value: 1)
+
+    expect(model.pluck(:value)).to contain_exactly(1)
+  end
+end

spec/tasks/cache/clear/redis_spec.rb

@@ -2,13 +2,16 @@
 
 require 'rake_helper'
 
-RSpec.describe 'clearing redis cache', :clean_gitlab_redis_cache, :silence_stdout do
+RSpec.describe 'clearing redis cache', :clean_gitlab_redis_repository_cache, :clean_gitlab_redis_cache,
+               :silence_stdout, feature_category: :redis do
   before do
     Rake.application.rake_require 'tasks/cache'
   end
 
+  let(:keys_size_changed) { -1 }
+
   shared_examples 'clears the cache' do
-    it { expect { run_rake_task('cache:clear:redis') }.to change { redis_keys.size }.by(-1) }
+    it { expect { run_rake_task('cache:clear:redis') }.to change { redis_keys.size }.by(keys_size_changed) }
   end
 
   describe 'clearing pipeline status cache' do
@@ -17,15 +20,37 @@ RSpec.describe 'clearing redis cache', :clean_gitlab_redis_cache, :silence_stdou
       create(:ci_pipeline, project: project).project.pipeline_status
     end
 
-    before do
-      allow(pipeline_status).to receive(:loaded).and_return(nil)
+    context 'when use_primary_and_secondary_stores_for_repository_cache MultiStore FF is enabled' do
+      # Initially, project:{id}:pipeline_status is explicitly cached in Gitlab::Redis::Cache, whereas repository is
+      # cached in Rails.cache (which is a NullStore).
+      # With the MultiStore feature flag enabled, we use Gitlab::Redis::RepositoryCache instance as primary store and
+      # Gitlab::Redis::Cache as secondary store.
+      # This ends up storing 2 extra keys (exists? and root_ref) in both Gitlab::Redis::RepositoryCache and
+      # Gitlab::Redis::Cache instances when loading project.pipeline_status
+      let(:keys_size_changed) { -3 }
+
+      before do
+        stub_feature_flags(use_primary_and_secondary_stores_for_repository_cache: true)
+        allow(pipeline_status).to receive(:loaded).and_return(nil)
+      end
+
+      it 'clears pipeline status cache' do
+        expect { run_rake_task('cache:clear:redis') }.to change { pipeline_status.has_cache? }
+      end
+
+      it_behaves_like 'clears the cache'
     end
 
-    it 'clears pipeline status cache' do
-      expect { run_rake_task('cache:clear:redis') }.to change { pipeline_status.has_cache? }
-    end
+    context 'when use_primary_and_secondary_stores_for_repository_cache and
+            use_primary_store_as_default_for_repository_cache feature flags are disabled' do
+      before do
+        stub_feature_flags(use_primary_and_secondary_stores_for_repository_cache: false)
+        stub_feature_flags(use_primary_store_as_default_for_repository_cache: false)
+        allow(pipeline_status).to receive(:loaded).and_return(nil)
+      end
 
-    it_behaves_like 'clears the cache'
+      it_behaves_like 'clears the cache'
+    end
   end
 
   describe 'clearing set caches' do

spec/views/projects/commit/show.html.haml_spec.rb

@@ -102,7 +102,7 @@ RSpec.describe 'projects/commit/show.html.haml', feature_category: :source_code
 
       it 'renders unverified badge' do
         expect(title).to include('This commit was signed with an unverified signature.')
-        expect(content).to match(/SSH key fingerprint:[\s\S]+Unknown/)
+        expect(content).to match(/SSH key fingerprint:[\s\S].+#{commit.signature.key_fingerprint_sha256}/)
       end
     end