Merge branch '9841-geo-unable-to-compare-branches-on-secondary' into 'master'

Allow users to compare Git revisions on a read-only instance

Closes #9841

See merge request gitlab-org/gitlab!18038

app/views/projects/branches/_branch.html.haml

@@ -34,8 +34,9 @@
         = _('Merge request')
 
     - if branch.name != @repository.root_ref
-      = link_to project_compare_path(@project, @repository.root_ref, branch.name),
+      = link_to project_compare_index_path(@project, from: @repository.root_ref, to: branch.name),
         class: "btn btn-default js-onboarding-compare-branches #{'prepend-left-10' unless merge_project}",
+        method: :post,
         title: s_('Branches|Compare') do
         = s_('Branches|Compare')
 

changelogs/unreleased/9841-geo-unable-to-compare-branches-on-secondary.yml

+---
+title: Allow users to compare Git revisions on a read-only instance
+merge_request: 18038
+author:
+type: fixed

lib/gitlab/middleware/read_only/controller.rb

@@ -20,6 +20,10 @@ module Gitlab
           'projects/lfs_locks_api' => %w{verify create unlock}
         }.freeze
 
+        WHITELISTED_GIT_REVISION_ROUTES = {
+          'projects/compare' => %w{create}
+        }.freeze
+
         GRAPHQL_URL = '/api/graphql'
 
         def initialize(app, env)
@@ -81,7 +85,7 @@ module Gitlab
 
         # Overridden in EE module
         def whitelisted_routes
-          grack_route? || internal_route? || lfs_route? || sidekiq_route? || graphql_query?
+          grack_route? || internal_route? || lfs_route? || compare_git_revisions_route? || sidekiq_route? || graphql_query?
         end
 
         def grack_route?
@@ -96,6 +100,13 @@ module Gitlab
           ReadOnly.internal_routes.any? { |path| request.path.include?(path) }
         end
 
+        def compare_git_revisions_route?
+          # Calling route_hash may be expensive. Only do it if we think there's a possible match
+          return false unless request.post? && request.path.end_with?('compare')
+
+          WHITELISTED_GIT_REVISION_ROUTES[route_hash[:controller]]&.include?(route_hash[:action])
+        end
+
         def lfs_route?
           # Calling route_hash may be expensive. Only do it if we think there's a possible match
           unless request.path.end_with?('/info/lfs/objects/batch',

spec/features/projects/branches_spec.rb

@@ -246,7 +246,6 @@ describe 'Branches' do
         end
 
         expect(page).to have_content 'Commits'
-        expect(page).to have_link 'Create merge request'
       end
     end
 

spec/features/projects/compare_spec.rb

@@ -12,6 +12,23 @@ describe "Compare", :js do
   end
 
   describe "branches" do
+    shared_examples 'compares branches' do
+      it 'compares branches' do
+        visit project_compare_index_path(project, from: 'master', to: 'master')
+
+        select_using_dropdown 'from', 'feature'
+        expect(find('.js-compare-from-dropdown .dropdown-toggle-text')).to have_content('feature')
+
+        select_using_dropdown 'to', 'binary-encoding'
+        expect(find('.js-compare-to-dropdown .dropdown-toggle-text')).to have_content('binary-encoding')
+
+        click_button 'Compare'
+
+        expect(page).to have_content 'Commits'
+        expect(page).to have_link 'Create merge request'
+      end
+    end
+
     it "pre-populates fields" do
       visit project_compare_index_path(project, from: "master", to: "master")
 
@@ -19,19 +36,14 @@ describe "Compare", :js do
       expect(find(".js-compare-to-dropdown .dropdown-toggle-text")).to have_content("master")
     end
 
-    it "compares branches" do
-      visit project_compare_index_path(project, from: "master", to: "master")
-
-      select_using_dropdown "from", "feature"
-      expect(find(".js-compare-from-dropdown .dropdown-toggle-text")).to have_content("feature")
-
-      select_using_dropdown "to", "binary-encoding"
-      expect(find(".js-compare-to-dropdown .dropdown-toggle-text")).to have_content("binary-encoding")
+    it_behaves_like 'compares branches'
 
-      click_button "Compare"
+    context 'on a read-only instance' do
+      before do
+        allow(Gitlab::Database).to receive(:read_only?).and_return(true)
+      end
 
-      expect(page).to have_content "Commits"
-      expect(page).to have_link 'Create merge request'
+      it_behaves_like 'compares branches'
     end
 
     it 'renders additions info when click unfold diff' do