Merge branch...

Merge branch '9217-warn-on-git-fetch-over-ssh-if-the-secondary-is-lagging-the-primary' into 'master'

Geo: Display secondary replication lag on console (if lag > 0 seconds)

Closes #9217

See merge request gitlab-org/gitlab-ee!10471

ee/changelogs/unreleased/9217-warn-on-git-fetch-over-ssh-if-the-secondary-is-lagging-the-primary.yml

+---
+title: 'Geo: Display secondary replication lag on console (if lag > 0 seconds)'
+merge_request: 10471
+author:
+type: added

ee/lib/ee/gitlab/geo_git_access.rb

@@ -18,6 +18,13 @@ module EE
         super
       end
 
+      override :check_for_console_messages
+      def check_for_console_messages(cmd)
+        super.push(
+          *current_replication_lag_message(cmd)
+        )
+      end
+
       protected
 
       def project_or_wiki
@@ -26,8 +33,20 @@ module EE
 
       private
 
+      def current_replication_lag_message(cmd)
+        return unless upload_pack?(cmd) # git fetch / pull
+        return unless ::Gitlab::Database.read_only?
+        return unless current_replication_lag > 0
+
+        "Current replication lag: #{current_replication_lag} seconds"
+      end
+
+      def current_replication_lag
+        @current_replication_lag ||= ::Gitlab::Geo::HealthCheck.new.db_replication_lag_seconds
+      end
+
       def custom_action_for?(cmd)
-        return unless receive_pack?(cmd)
+        return unless receive_pack?(cmd) # git push
         return unless ::Gitlab::Database.read_only?
 
         ::Gitlab::Geo.secondary_with_primary?
@@ -76,7 +95,7 @@ module EE
       end
 
       def proxying_to_primary_message
-        ::Gitlab::Geo::GitPushSSHProxy.inform_client_message(primary_ssh_url_to_repo)
+        "You're pushing to a Geo secondary.\nWe'll help you by proxying this request to the primary: #{primary_ssh_url_to_repo}"
       end
 
       def custom_action_api_endpoints

ee/lib/gitlab/geo/git_push_ssh_proxy.rb

@@ -50,10 +50,6 @@ module Gitlab
         @data = data
       end
 
-      def self.inform_client_message(primary_repo_ssh)
-        "You're pushing to a Geo secondary.\nWe'll help you by proxying this request to the primary: #{primary_repo_ssh}"
-      end
-
       def info_refs
         ensure_secondary!
 

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

@@ -35,12 +35,6 @@ describe Gitlab::Geo::GitPushSSHProxy, :geo do
     }
   end
 
-  describe '.inform_client_message' do
-    it 'returns a message, with the ssh address' do
-      expect(described_class.inform_client_message(primary_repo_ssh)).to eql("You're pushing to a Geo secondary.\nWe'll help you by proxying this request to the primary: #{primary_repo_ssh}")
-    end
-  end
-
   context 'instance methods' do
     subject { described_class.new(data) }
 

ee/spec/lib/gitlab/git_access_spec.rb

 require 'spec_helper'
 
 describe Gitlab::GitAccess do
+  include EE::GeoHelpers
+
   set(:user) { create(:user) }
 
   let(:actor) { user }
@@ -249,11 +251,43 @@ describe Gitlab::GitAccess do
     end
   end
 
-  describe 'Geo system permissions' do
+  describe 'Geo' do
     let(:actor) { :geo }
 
-    it { expect { pull_changes }.not_to raise_error }
-    it { expect { push_changes }.to raise_unauthorized(Gitlab::GitAccess::ERROR_MESSAGES[:upload]) }
+    context 'git pull' do
+      it { expect { pull_changes }.not_to raise_error }
+
+      context 'for a secondary' do
+        let(:current_replication_lag) { nil }
+
+        before do
+          stub_licensed_features(geo: true)
+          stub_current_geo_node(create(:geo_node))
+
+          allow_any_instance_of(Gitlab::Geo::HealthCheck).to receive(:db_replication_lag_seconds).and_return(current_replication_lag)
+        end
+
+        context 'that has no DB replication lag' do
+          let(:current_replication_lag) { 0 }
+
+          it 'does not return a replication lag message in the console messages' do
+            expect(pull_changes.console_messages).to be_empty
+          end
+        end
+
+        context 'that has DB replication lag > 0' do
+          let(:current_replication_lag) { 7 }
+
+          it 'returns a replication lag message in the console messages' do
+            expect(pull_changes.console_messages).to eq(['Current replication lag: 7 seconds'])
+          end
+        end
+      end
+    end
+
+    context 'git push' do
+      it { expect { push_changes }.to raise_unauthorized(Gitlab::GitAccess::ERROR_MESSAGES[:upload]) }
+    end
   end
 
   private

ee/spec/requests/git_http_geo_spec.rb

 require 'spec_helper'
 
-describe "Git HTTP requests (Geo)" do
+describe "Git HTTP requests (Geo)", :geo do
   include TermsHelper
   include ::EE::GeoHelpers
   include GitHttpHelpers

lib/api/internal.rb

@@ -87,7 +87,8 @@ module API
             gl_id: Gitlab::GlId.gl_id(user),
             gl_username: user&.username,
             git_config_options: [],
-            gitaly: gitaly_payload(params[:action])
+            gitaly: gitaly_payload(params[:action]),
+            gl_console_messages: check_result.console_messages
           }
 
           # Custom option for git-receive-pack command

lib/gitlab/git_access.rb

@@ -85,7 +85,7 @@ module Gitlab
         check_push_access!
       end
 
-      ::Gitlab::GitAccessResult::Success.new
+      ::Gitlab::GitAccessResult::Success.new(console_messages: check_for_console_messages(cmd))
     end
 
     def guest_can_download_code?
@@ -116,6 +116,10 @@ module Gitlab
       nil
     end
 
+    def check_for_console_messages(cmd)
+      []
+    end
+
     def check_valid_actor!
       return unless actor.is_a?(Key)
 

lib/gitlab/git_access_result/success.rb

@@ -3,6 +3,11 @@
 module Gitlab
   module GitAccessResult
     class Success
+      attr_reader :console_messages
+
+      def initialize(console_messages: [])
+        @console_messages = console_messages
+      end
     end
   end
 end

spec/requests/api/internal_spec.rb

@@ -498,6 +498,40 @@ describe API::Internal do
       end
     end
 
+    context "console message" do
+      before do
+        project.add_developer(user)
+      end
+
+      context "git pull" do
+        context "with no console message" do
+          it "has the correct payload" do
+            pull(key, project)
+
+            expect(response).to have_gitlab_http_status(200)
+            expect(json_response['gl_console_messages']).to eq([])
+          end
+        end
+
+        context "with a console message" do
+          let(:console_messages) { ['message for the console'] }
+
+          it "has the correct payload" do
+            expect_next_instance_of(Gitlab::GitAccess) do |access|
+              expect(access).to receive(:check_for_console_messages)
+                                  .with('git-upload-pack')
+                                  .and_return(console_messages)
+            end
+
+            pull(key, project)
+
+            expect(response).to have_gitlab_http_status(200)
+            expect(json_response['gl_console_messages']).to eq(console_messages)
+          end
+        end
+      end
+    end
+
     context "blocked user" do
       let(:personal_project) { create(:project, namespace: user.namespace) }