Merge branch 'mk/apply-selective-sync-to-package-file-events' into 'master'

Geo: Package file replication should adhere to selective sync settings

Closes #213889

See merge request gitlab-org/gitlab!32364

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

@@ -21,6 +21,8 @@ module Geo
 
     # Called by Gitlab::Geo::Replicator#consume
     def consume_event_created(**params)
+      return if excluded_by_selective_sync?
+
       download
     end
 

ee/lib/gitlab/geo/replicator.rb

@@ -10,6 +10,7 @@ module Gitlab
     #
     # Each replicator is tied to a specific replicable resource
     class Replicator
+      include ::Gitlab::Utils::StrongMemoize
       include ::Gitlab::Geo::LogHelpers
       extend ::Gitlab::Geo::LogHelpers
 
@@ -212,6 +213,26 @@ module Gitlab
         registry.verification_checksum
       end
 
+      # This method does not yet cover resources that are owned by a namespace
+      # but not a project, because we do not have that use-case...yet.
+      # E.g. GroupWikis will need it.
+      def excluded_by_selective_sync?
+        # If the replicable is not owned by a project or namespace, then selective sync cannot apply to it.
+        return false unless parent_project_id
+
+        !current_node.projects_include?(parent_project_id)
+      end
+
+      def parent_project_id
+        strong_memoize(:parent_project_id) do
+          # We should never see this at runtime. All Replicators should be tested
+          # by `it_behaves_like 'a replicator'`, which would reveal this problem.
+          selective_sync_not_implemented_error(__method__) unless model_record.respond_to?(:project_id)
+
+          model_record.project_id
+        end
+      end
+
       protected
 
       # Store an event on the database
@@ -236,6 +257,15 @@ module Gitlab
       rescue ActiveRecord::RecordInvalid, NoMethodError => e
         log_error("#{class_name} could not be created", e, params)
       end
+
+      def current_node
+        Gitlab::Geo.current_node
+      end
+
+      def selective_sync_not_implemented_error(method_name)
+        raise NotImplementedError,
+            "#{self.class} does not implement #{method_name}. If selective sync is not applicable, just return nil."
+      end
     end
   end
 end

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

@@ -178,5 +178,67 @@ describe Gitlab::Geo::Replicator do
         end
       end
     end
+
+    describe '#excluded_by_selective_sync?' do
+      subject(:replicator) { Geo::DummyReplicator.new }
+
+      before do
+        stub_current_geo_node(secondary_node)
+      end
+
+      context 'when parent_project_id is not nil' do
+        before do
+          allow(replicator).to receive(:parent_project_id).and_return(123456)
+        end
+
+        context 'when the current Geo node excludes the parent_project due to selective sync' do
+          it 'returns true' do
+            expect(secondary_node).to receive(:projects_include?).with(123456).and_return(false)
+
+            expect(replicator.excluded_by_selective_sync?).to eq(true)
+          end
+        end
+
+        context 'when the current Geo node does not exclude the parent_project due to selective sync' do
+          it 'returns false' do
+            expect(secondary_node).to receive(:projects_include?).with(123456).and_return(true)
+
+            expect(replicator.excluded_by_selective_sync?).to eq(false)
+          end
+        end
+      end
+
+      context 'when parent_project_id is nil' do
+        before do
+          expect(replicator).to receive(:parent_project_id).and_return(nil)
+        end
+
+        it 'returns false' do
+          expect(replicator.excluded_by_selective_sync?).to eq(false)
+        end
+      end
+    end
+
+    describe '#parent_project_id' do
+      subject(:replicator) { Geo::DummyReplicator.new(model_record: model_record) }
+
+      # We cannot infer parent project, so parent_project_id should be overridden.
+      context 'when model_record does not respond to project_id' do
+        let(:model_record) { double(:model_record) }
+
+        it 'raises NotImplementedError' do
+          expect { replicator.parent_project_id }.to raise_error(NotImplementedError)
+        end
+      end
+
+      # We assume project_id to be the parent project.
+      context 'when model_record responds to project_id' do
+        let(:model_record) { double(:model_record, project_id: 1234) }
+
+        it 'does not error' do
+          expect(replicator.parent_project_id).to eq(1234)
+        end
+      end
+    end
   end
 end

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

@@ -18,6 +18,8 @@ RSpec.shared_examples 'a blob replicator' do
     stub_current_geo_node(primary)
   end
 
+  it_behaves_like 'a replicator'
+
   describe '#handle_after_create_commit' do
     it 'creates a Geo::Event' do
       expect do
@@ -70,13 +72,26 @@ RSpec.shared_examples 'a blob replicator' do
   end
 
   describe '#consume_created_event' do
-    it 'invokes Geo::BlobDownloadService' do
-      service = double(:service)
+    context "when the blob's project is not excluded by selective sync" do
+      it 'invokes Geo::BlobDownloadService' do
+        expect(replicator).to receive(:excluded_by_selective_sync?).and_return(false)
+        service = double(:service)
+
+        expect(service).to receive(:execute)
+        expect(::Geo::BlobDownloadService).to receive(:new).with(replicator: replicator).and_return(service)
 
-      expect(service).to receive(:execute)
-      expect(::Geo::BlobDownloadService).to receive(:new).with(replicator: replicator).and_return(service)
+        replicator.consume_event_created
+      end
+    end
 
-      replicator.consume_event_created
+    context "when the blob's project is excluded by selective sync" do
+      it 'does not invoke Geo::BlobDownloadService' do
+        expect(replicator).to receive(:excluded_by_selective_sync?).and_return(true)
+
+        expect(::Geo::BlobDownloadService).not_to receive(:new)
+
+        replicator.consume_event_created
+      end
     end
   end
 

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

+# frozen_string_literal: true
+
+# Include these shared examples in BlobReplicatorStrategy,
+# RepositoryReplicatorStrategy, etc.
+#
+# Required let variables:
+#
+#   - `replicator` should be an instance of the Replicator class being tested, e.g. PackageFileReplicator
+#   - `model_record` should be a valid instance of the model class. It may be unpersisted.
+#   - `primary` should be the primary GeoNode
+#   - `secondary` should be a secondary GeoNode
+#
+RSpec.shared_examples 'a replicator' do
+  include EE::GeoHelpers
+
+  describe '#parent_project_id' do
+    it 'is implemented if needed' do
+      expect { replicator.parent_project_id }.not_to raise_error
+    end
+  end
+end