diff --git a/ee/app/controllers/ee/repositories/git_http_client_controller.rb b/ee/app/controllers/ee/repositories/git_http_client_controller.rb index e3309d129abe009b4f6e2c26a428a2bb85d402eb..ae05e229865dd0a2f8a5b9cb55357e9f063a37fc 100644 --- a/ee/app/controllers/ee/repositories/git_http_client_controller.rb +++ b/ee/app/controllers/ee/repositories/git_http_client_controller.rb @@ -84,7 +84,7 @@ def redirect? def out_of_date_redirect? return false unless project - git_upload_pack_request? && ::Geo::ProjectRegistry.repository_out_of_date?(project.id) + git_upload_pack_request? && repository_out_of_date?(project) end private @@ -136,6 +136,14 @@ def info_refs_request? def redirect_to_avoid_enumeration? project.nil? end + + def repository_out_of_date?(project) + if ::Geo::ProjectRepositoryReplicator.enabled? + ::Geo::ProjectRepositoryRegistry.repository_out_of_date?(project.id) + else + ::Geo::ProjectRegistry.repository_out_of_date?(project.id) + end + end end class GeoGitLFSHelper @@ -208,7 +216,11 @@ def batch_out_of_date? return !::Geo::LfsObjectRegistry.oids_synced?(requested_oids) end - ::Geo::ProjectRegistry.repository_out_of_date?(project.id) + if ::Geo::ProjectRepositoryReplicator.enabled? + ::Geo::ProjectRepositoryRegistry.repository_out_of_date?(project.id) + else + ::Geo::ProjectRegistry.repository_out_of_date?(project.id) + end end def wanted_version diff --git a/ee/app/models/geo/project_repository_registry.rb b/ee/app/models/geo/project_repository_registry.rb index 05343504fb50d861db9a9990e0a82b9ddd056c25..8be294a77b5b0d0c6f130d39ceab15aa854b182c 100644 --- a/ee/app/models/geo/project_repository_registry.rb +++ b/ee/app/models/geo/project_repository_registry.rb @@ -12,5 +12,37 @@ class ProjectRepositoryRegistry < Geo::BaseRegistry ignore_column :force_to_redownload, remove_with: '16.4', remove_after: '2023-08-22' belongs_to :project, class_name: 'Project' + + # @return [Boolean] whether the project repository is out-of-date on this site + def self.repository_out_of_date?(project_id) + return false unless ::Gitlab::Geo.secondary_with_primary? + + registry = find_by(project_id: project_id) + + # Out-of-date if registry or project don't exist + return true if registry.nil? || registry.project.nil? + + # Out-of-date if sync failed + return true if registry.failed? + + # Up-to-date if there is no timestamp for the latest change to the repo + return false unless registry.project.last_repository_updated_at + + # Out-of-date if the repo has never been synced + return true unless registry.last_synced_at + + # Return whether the latest change is replicated + # + # Current limitations: + # + # - We assume last_repository_updated_at is a timestamp of the latest change + # - last_repository_updated_at is also touched when a project wiki is updated + # - last_repository_updated_at touches are throttled within Event::REPOSITORY_UPDATED_AT_INTERVAL minutes + last_updated_at = registry.project.repository_state&.last_repository_updated_at + last_updated_at ||= registry.project.last_repository_updated_at + last_synced_at = registry.last_synced_at + + last_synced_at <= last_updated_at + end end end diff --git a/ee/lib/ee/gitlab/geo_git_access.rb b/ee/lib/ee/gitlab/geo_git_access.rb index 7749650532bf2fb933841467de4b71d40ae4ee40..c449aa792f5c7ae81b579de797d128f84f5a32e6 100644 --- a/ee/lib/ee/gitlab/geo_git_access.rb +++ b/ee/lib/ee/gitlab/geo_git_access.rb @@ -37,7 +37,15 @@ def geo_custom_action? def upload_pack_and_out_of_date? return false unless project - upload_pack? && ::Geo::ProjectRegistry.repository_out_of_date?(project.id) + upload_pack? && geo_repository_out_of_date?(project) + end + + def geo_repository_out_of_date?(project) + if ::Geo::ProjectRepositoryReplicator.enabled? + ::Geo::ProjectRepositoryRegistry.repository_out_of_date?(project.id) + else + ::Geo::ProjectRegistry.repository_out_of_date?(project.id) + end end def proxy_direct_to_primary_headers diff --git a/ee/spec/models/geo/project_repository_registry_spec.rb b/ee/spec/models/geo/project_repository_registry_spec.rb index 3c8b51ab0ccaa9d88521f8f481322d6ef22f664a..6578521d96bd464162037f24fe1292fda4c8cb31 100644 --- a/ee/spec/models/geo/project_repository_registry_spec.rb +++ b/ee/spec/models/geo/project_repository_registry_spec.rb @@ -3,11 +3,115 @@ require 'spec_helper' RSpec.describe Geo::ProjectRepositoryRegistry, :geo, type: :model, feature_category: :geo_replication do - let_it_be(:registry) { build(:geo_project_registry) } + include ::EE::GeoHelpers + + let_it_be(:registry) { build(:geo_project_repository_registry) } specify 'factory is valid' do expect(registry).to be_valid end include_examples 'a Geo framework registry' + + describe '.repository_out_of_date?' do + let_it_be(:project) { create(:project) } + + context 'for a non-Geo setup' do + it 'returns false' do + expect(described_class.repository_out_of_date?(project.id)).to be_falsey + end + end + + context 'for a Geo setup' do + before do + stub_current_geo_node(current_node) + end + + context 'for a Geo Primary' do + let(:current_node) { create(:geo_node, :primary) } + + it 'returns false' do + expect(described_class.repository_out_of_date?(project.id)).to be_falsey + end + end + + context 'for a Geo secondary' do + let(:current_node) { create(:geo_node) } + + context 'when Primary node is not configured' do + it 'returns false' do + expect(described_class.repository_out_of_date?(project.id)).to be_falsey + end + end + + context 'when Primary node is configured' do + before do + create(:geo_node, :primary) + end + + context 'when project_repository_registry entry does not exist' do + it 'returns true' do + expect(described_class.repository_out_of_date?(project.id)).to be_truthy + end + end + + context 'when project_repository_registry entry does exist' do + context 'when last_repository_updated_at is not set' do + it 'returns false' do + registry = create(:geo_project_repository_registry, :synced) + registry.project.update!(last_repository_updated_at: nil) + + expect(described_class.repository_out_of_date?(registry.project_id)).to be_falsey + end + end + + context 'when last_repository_updated_at is set' do + context 'when sync failed' do + it 'returns true' do + registry = create(:geo_project_repository_registry, :failed) + + expect(described_class.repository_out_of_date?(registry.project_id)).to be_truthy + end + end + + context 'when last_synced_at is not set' do + it 'returns true' do + registry = create(:geo_project_repository_registry, last_synced_at: nil) + + expect(described_class.repository_out_of_date?(registry.project_id)).to be_truthy + end + end + + context 'when last_synced_at is set', :freeze_time do + using RSpec::Parameterized::TableSyntax + + let_it_be(:project_repository_state) { create(:repository_state, project: project) } + + where(:project_last_updated, :project_state_last_updated, :project_registry_last_synced, :expected) do + (Time.current - 3.minutes) | nil | (Time.current - 5.minutes) | true + (Time.current - 3.minutes) | nil | (Time.current - 1.minute) | false + (Time.current - 3.minutes) | Time.current | (Time.current - 1.minute) | true + (Time.current - 3.minutes) | (Time.current - 2.minutes) | (Time.current - 1.minute) | false + (Time.current - 3.minutes) | Time.current | (Time.current - 5.minutes) | true + end + + with_them do + before do + project.update!(last_repository_updated_at: project_last_updated) + project_repository_state.update!(last_repository_updated_at: project_state_last_updated) + create(:geo_project_repository_registry, :synced, + project: project, last_synced_at: project_registry_last_synced) + end + + it 'returns the expected value' do + expect(described_class.repository_out_of_date?(project.id)).to eq(expected) + end + end + end + end + end + end + end + end + end end diff --git a/ee/spec/requests/git_http_geo_spec.rb b/ee/spec/requests/git_http_geo_spec.rb index afd23b3c8c8c9070c791b1dcc18ef52d56f5dfea..d89c446a1536d68df42abb45c98c7476f9662576 100644 --- a/ee/spec/requests/git_http_geo_spec.rb +++ b/ee/spec/requests/git_http_geo_spec.rb @@ -12,6 +12,7 @@ let_it_be(:project_with_repo) { create(:project, :repository, :private) } let_it_be(:project_no_repo) { create(:project, :private) } let_it_be(:registry_with_repo) { create(:geo_project_registry, :synced, project: project_with_repo, last_repository_successful_sync_at: project_with_repo.last_repository_updated_at + 10.seconds) } + let_it_be(:project_registry_with_repo) { create(:geo_project_repository_registry, :synced, project: project_with_repo, last_synced_at: project_with_repo.last_repository_updated_at + 10.seconds) } let_it_be(:primary_url) { 'http://primary.example.com' } let_it_be(:primary_internal_url) { 'http://primary-internal.example.com' } @@ -145,34 +146,78 @@ def make_request let(:endpoint_path) { "/#{project_with_repo_but_not_synced.full_path}.git/info/refs?service=git-upload-pack" } let(:redirect_url) { redirected_primary_url } - before do - create(:geo_project_registry, :synced, project: project_with_repo_but_not_synced, last_repository_successful_sync_at: nil) - end + context 'with geo_project_repository_replication feature flag disabled' do + before do + stub_feature_flags(geo_project_repository_replication: false) + create(:geo_project_registry, :synced, project: project_with_repo_but_not_synced, last_repository_successful_sync_at: nil) + end - it_behaves_like 'a Geo 302 redirect to Primary' + it_behaves_like 'a Geo 302 redirect to Primary' - context 'when terms are enforced' do + context 'when terms are enforced' do + before do + enforce_terms + end + + it_behaves_like 'a Geo 302 redirect to Primary' + end + end + + context 'with geo_project_repository_replication feature flag enabled' do before do - enforce_terms + stub_feature_flags(geo_project_repository_replication: true) + create(:geo_project_repository_registry, :synced, project: project_with_repo_but_not_synced, last_synced_at: nil) end it_behaves_like 'a Geo 302 redirect to Primary' + + context 'when terms are enforced' do + before do + enforce_terms + end + + it_behaves_like 'a Geo 302 redirect to Primary' + end end end context 'and has successfully synced' do let_it_be(:project) { project_with_repo } - it_behaves_like 'a Geo git request' - it_behaves_like 'a Geo 200 git request' + context 'with geo_project_repository_replication feature flag disabled' do + before do + stub_feature_flags(geo_project_repository_replication: false) + end - context 'when terms are enforced' do + it_behaves_like 'a Geo git request' + it_behaves_like 'a Geo 200 git request' + + context 'when terms are enforced' do + before do + enforce_terms + end + + it_behaves_like 'a Geo git request' + it_behaves_like 'a Geo 200 git request' + end + end + + context 'with geo_project_repository_replication feature flag enabled' do before do - enforce_terms + stub_feature_flags(geo_project_repository_replication: true) end it_behaves_like 'a Geo git request' it_behaves_like 'a Geo 200 git request' + + context 'when terms are enforced' do + before do + enforce_terms + end + + it_behaves_like 'a Geo git request' + it_behaves_like 'a Geo 200 git request' + end end end end @@ -183,14 +228,36 @@ def make_request let(:endpoint_path) { "/#{project.full_path}.git/info/refs?service=git-upload-pack" } let(:redirect_url) { redirected_primary_url } - it_behaves_like 'a Geo 302 redirect to Primary' + context 'with geo_project_repository_replication feature flag disabled' do + before do + stub_feature_flags(geo_project_repository_replication: false) + end - context 'when terms are enforced' do + it_behaves_like 'a Geo 302 redirect to Primary' + + context 'when terms are enforced' do + before do + enforce_terms + end + + it_behaves_like 'a Geo 302 redirect to Primary' + end + end + + context 'with geo_project_repository_replication feature flag enabled' do before do - enforce_terms + stub_feature_flags(geo_project_repository_replication: true) end it_behaves_like 'a Geo 302 redirect to Primary' + + context 'when terms are enforced' do + before do + enforce_terms + end + + it_behaves_like 'a Geo 302 redirect to Primary' + end end end @@ -206,16 +273,40 @@ def make_request let(:endpoint_path) { "/#{non_existent_project_path}.git/info/refs?service=git-upload-pack" } let(:redirect_url) { redirected_primary_url } - # To avoid enumeration of private projects not in selective sync, - # this response must be the same as a private project without a repo. - it_behaves_like 'a Geo 302 redirect to Primary' + context 'with geo_project_repository_replication feature flag disabled' do + before do + stub_feature_flags(geo_project_repository_replication: false) + end - context 'when terms are enforced' do + # To avoid enumeration of private projects not in selective sync, + # this response must be the same as a private project without a repo. + it_behaves_like 'a Geo 302 redirect to Primary' + + context 'when terms are enforced' do + before do + enforce_terms + end + + it_behaves_like 'a Geo 302 redirect to Primary' + end + end + + context 'with geo_project_repository_replication feature flag enabled' do before do - enforce_terms + stub_feature_flags(geo_project_repository_replication: true) end + # To avoid enumeration of private projects not in selective sync, + # this response must be the same as a private project without a repo. it_behaves_like 'a Geo 302 redirect to Primary' + + context 'when terms are enforced' do + before do + enforce_terms + end + + it_behaves_like 'a Geo 302 redirect to Primary' + end end end end @@ -230,12 +321,31 @@ def make_request let(:endpoint_path) { "/#{project.full_path}.git/info/refs" } let(:redirect_url) { "#{redirected_primary_url}?service=git-receive-pack" } - subject do - make_request - response + context 'with geo_project_repository_replication feature flag disabled' do + before do + stub_feature_flags(geo_project_repository_replication: false) + end + + subject do + make_request + response + end + + it_behaves_like 'a Geo 302 redirect to Primary' end - it_behaves_like 'a Geo 302 redirect to Primary' + context 'with geo_project_repository_replication feature flag enabled' do + before do + stub_feature_flags(geo_project_repository_replication: true) + end + + subject do + make_request + response + end + + it_behaves_like 'a Geo 302 redirect to Primary' + end end end @@ -247,16 +357,40 @@ def make_request context 'when the repository exists' do let_it_be(:project) { project_with_repo } - it_behaves_like 'a Geo git request' - it_behaves_like 'a Geo 200 git request' + context 'with geo_project_repository_replication feature flag disabled' do + before do + stub_feature_flags(geo_project_repository_replication: false) + end + + it_behaves_like 'a Geo git request' + it_behaves_like 'a Geo 200 git request' + + context 'when terms are enforced' do + before do + enforce_terms + end + + it_behaves_like 'a Geo git request' + it_behaves_like 'a Geo 200 git request' + end + end - context 'when terms are enforced' do + context 'with geo_project_repository_replication feature flag enabled' do before do - enforce_terms + stub_feature_flags(geo_project_repository_replication: true) end it_behaves_like 'a Geo git request' it_behaves_like 'a Geo 200 git request' + + context 'when terms are enforced' do + before do + enforce_terms + end + + it_behaves_like 'a Geo git request' + it_behaves_like 'a Geo 200 git request' + end end end @@ -266,14 +400,36 @@ def make_request let(:endpoint_path) { "/#{project.full_path}.git/git-upload-pack" } let(:redirect_url) { redirected_primary_url } - it_behaves_like 'a Geo 302 redirect to Primary' + context 'with geo_project_repository_replication feature flag disabled' do + before do + stub_feature_flags(geo_project_repository_replication: false) + end + + it_behaves_like 'a Geo 302 redirect to Primary' + + context 'when terms are enforced' do + before do + enforce_terms + end + + it_behaves_like 'a Geo 302 redirect to Primary' + end + end - context 'when terms are enforced' do + context 'with geo_project_repository_replication feature flag enabled' do before do - enforce_terms + stub_feature_flags(geo_project_repository_replication: true) end it_behaves_like 'a Geo 302 redirect to Primary' + + context 'when terms are enforced' do + before do + enforce_terms + end + + it_behaves_like 'a Geo 302 redirect to Primary' + end end end end @@ -334,6 +490,7 @@ def make_request end end + # rubocop:disable RSpec/MultipleMemoizedHelpers context 'operation download' do let(:user) { create(:user) } let(:authorization) { ActionController::HttpAuthentication::Basic.encode_credentials(user.username, user.password) } @@ -406,21 +563,63 @@ def make_request stub_feature_flags(geo_proxy_lfs_batch_requests: false) end - context 'out of sync' do + context 'with geo_project_repository_replication feature flag disabled' do before do - allow(::Geo::ProjectRegistry).to receive(:repository_out_of_date?).with(project.id).and_return(true) + stub_feature_flags(geo_project_repository_replication: false) end - it_behaves_like 'a Geo 302 redirect to Primary' + context 'out of sync' do + before do + allow(::Geo::ProjectRegistry) + .to receive(:repository_out_of_date?) + .with(project.id) + .and_return(true) + end + + it_behaves_like 'a Geo 302 redirect to Primary' + end + + context 'in sync' do + before do + allow(::Geo::ProjectRegistry) + .to receive(:repository_out_of_date?) + .with(project.id) + .and_return(false) + end + + it 'is handled by the secondary' do + is_expected.to have_gitlab_http_status(:ok) + end + end end - context 'in sync' do + context 'with geo_project_repository_replication feature flag enabled' do before do - allow(::Geo::ProjectRegistry).to receive(:repository_out_of_date?).with(project.id).and_return(false) + stub_feature_flags(geo_project_repository_replication: true) + end + + context 'out of sync' do + before do + allow(::Geo::ProjectRepositoryRegistry) + .to receive(:repository_out_of_date?) + .with(project.id) + .and_return(true) + end + + it_behaves_like 'a Geo 302 redirect to Primary' end - it 'is handled by the secondary' do - is_expected.to have_gitlab_http_status(:ok) + context 'in sync' do + before do + allow(::Geo::ProjectRepositoryRegistry) + .to receive(:repository_out_of_date?) + .with(project.id) + .and_return(false) + end + + it 'is handled by the secondary' do + is_expected.to have_gitlab_http_status(:ok) + end end end end @@ -447,6 +646,7 @@ def make_request end end end + # rubocop:enable RSpec/MultipleMemoizedHelpers end end @@ -485,12 +685,34 @@ def make_request context 'when the repository has been updated' do let_it_be(:project) { project_with_repo } - before do - allow(::Geo::ProjectRegistry).to receive(:repository_out_of_date?).with(project.id).and_return(true) + context 'with geo_project_repository_replication feature flag disabled' do + before do + stub_feature_flags(geo_project_repository_replication: false) + + allow(::Geo::ProjectRegistry) + .to receive(:repository_out_of_date?) + .with(project.id) + .and_return(true) + end + + it 'is handled by the secondary' do + is_expected.to have_gitlab_http_status(:ok) + end end - it 'is handled by the secondary' do - is_expected.to have_gitlab_http_status(:ok) + context 'with geo_project_repository_replication feature flag enabled' do + before do + stub_feature_flags(geo_project_repository_replication: true) + + allow(::Geo::ProjectRepositoryRegistry) + .to receive(:repository_out_of_date?) + .with(project.id) + .and_return(true) + end + + it 'is handled by the secondary' do + is_expected.to have_gitlab_http_status(:ok) + end end end end