Geo: Fix maintenance mode causing Unhealthy secondary status

The Geo node status endpoint was being blocked by the Geo primary when
maintenance mode was enabled. This commit adds the endpoint to the
allowlist.

Changelog: fixed
EE: true

ee/lib/ee/gitlab/middleware/read_only/controller.rb

@@ -35,13 +35,22 @@ module EE
 
           private
 
-          # In addition to routes allowed in FOSS, allow geo node update route
-          # and geo api route, on both Geo primary and secondary.
-          # If this is on a Geo secondary, also allow git write routes and custom logout routes.
-          # If in maintenance mode, don't allow git write routes on Geo secondary either
           override :allowlisted_routes
+
+          # In addition to routes allowed in FOSS, allow:
+          #
+          # - on both Geo primary and secondary
+          #    - geo node update route
+          #    - geo resync designs route
+          #    - geo custom sign_out route
+          # - on Geo primary
+          #    - geo node status update route
+          # - on Geo secondary with maintenance mode off
+          #    - git write routes
+          #
+          # @return [Boolean] true whether current route is in allow list.
           def allowlisted_routes
-            allowed = super || geo_node_update_route? || geo_api_route? || geo_sign_out_route? || admin_settings_update?
+            allowed = super || geo_node_update_route? || geo_resync_designs_route? || geo_sign_out_route? || admin_settings_update? || geo_node_status_update_route?
 
             return true if allowed
             return sign_in_route? if ::Gitlab.maintenance_mode?
@@ -87,7 +96,7 @@ module EE
             ALLOWLISTED_GIT_READ_WRITE_ROUTES[route_hash[:controller]]&.include?(route_hash[:action])
           end
 
-          def geo_api_route?
+          def geo_resync_designs_route?
             ::Gitlab::Middleware::ReadOnly::API_VERSIONS.any? do |version|
               request.path.include?("/api/v#{version}/geo_replication")
             end
@@ -99,6 +108,12 @@ module EE
             ALLOWLISTED_SIGN_OUT_ROUTES[route_hash[:controller]]&.include?(route_hash[:action])
           end
 
+          def geo_node_status_update_route?
+            ::Gitlab::Middleware::ReadOnly::API_VERSIONS.any? do |version|
+              request.path.include?("/api/v#{version}/geo/status")
+            end
+          end
+
           def sign_in_route?
             return unless request.post? && request.path.start_with?('/users/sign_in', '/oauth/token',
               '/users/auth/geo/sign_in')

ee/spec/support/shared_examples/lib/gitlab/middleware/maintenance_mode_gitlab_ee_instance_shared_examples.rb

@@ -2,8 +2,57 @@
 
 RSpec.shared_examples 'write access for a read-only GitLab (EE) instance in maintenance mode' do
   include Rack::Test::Methods
+  include EE::GeoHelpers
   using RSpec::Parameterized::TableSyntax
 
+  shared_examples_for 'LFS changes are disallowed' do
+    where(:description, :path) do
+      'LFS request to locks verify' | '/root/rouge.git/info/lfs/locks/verify'
+      'LFS request to locks create' | '/root/rouge.git/info/lfs/locks'
+      'LFS request to locks unlock' | '/root/rouge.git/info/lfs/locks/1/unlock'
+    end
+
+    with_them do
+      it "expects a POST #{description} URL not to be allowed" do
+        response = request.post(path)
+
+        expect(response).to be_redirect
+        expect(subject).to disallow_request
+      end
+
+      it "expects a POST #{description} URL with trailing backslash not to be allowed" do
+        response = request.post("#{path}/")
+
+        expect(response).to be_redirect
+        expect(subject).to disallow_request
+      end
+    end
+  end
+
+  shared_examples_for 'sign in/out and OAuth are allowed' do
+    where(:description, :path) do
+      'sign in route'     | '/users/sign_in'
+      'sign out route'    | '/users/sign_out'
+      'oauth token route' | '/oauth/token'
+    end
+
+    with_them do
+      it "expects a POST to #{description} URL to be allowed" do
+        response = request.post(path)
+
+        expect(response).not_to be_redirect
+        expect(subject).not_to disallow_request
+      end
+
+      it "expects a POST to #{description} URL with trailing slash to be allowed" do
+        response = request.post("#{path}/")
+
+        expect(response).not_to be_redirect
+        expect(subject).not_to disallow_request
+      end
+    end
+  end
+
   include_context 'with a mocked GitLab instance'
 
   before do
@@ -29,9 +78,30 @@ RSpec.shared_examples 'write access for a read-only GitLab (EE) instance in main
       expect(subject).not_to disallow_request
     end
 
+    context 'without Geo enabled' do
+      it_behaves_like 'LFS changes are disallowed'
+      it_behaves_like 'sign in/out and OAuth are allowed'
+    end
+
+    context 'on Geo primary' do
+      before do
+        stub_primary_node
+      end
+
+      it_behaves_like 'LFS changes are disallowed'
+      it_behaves_like 'sign in/out and OAuth are allowed'
+
+      it "allows Geo node status updates from Geo secondaries" do
+        response = request.post("/api/#{API::API.version}/geo/status")
+
+        expect(response).not_to be_redirect
+        expect(subject).not_to disallow_request
+      end
+    end
+
     context 'on Geo secondary' do
       before do
-        allow(::Gitlab::Geo).to receive(:secondary?).and_return(true)
+        stub_secondary_node
       end
 
       where(:description, :path) do
@@ -88,55 +158,5 @@ RSpec.shared_examples 'write access for a read-only GitLab (EE) instance in main
         expect(subject).to disallow_request
       end
     end
-
-    context 'when not on Geo secondary' do
-      before do
-        allow(::Gitlab::Geo).to receive(:secondary?).and_return(false)
-      end
-
-      where(:description, :path) do
-        'LFS request to locks verify' | '/root/rouge.git/info/lfs/locks/verify'
-        'LFS request to locks create' | '/root/rouge.git/info/lfs/locks'
-        'LFS request to locks unlock' | '/root/rouge.git/info/lfs/locks/1/unlock'
-      end
-
-      with_them do
-        it "expects a POST #{description} URL not to be allowed" do
-          response = request.post(path)
-
-          expect(response).to be_redirect
-          expect(subject).to disallow_request
-        end
-
-        it "expects a POST #{description} URL with trailing backslash not to be allowed" do
-          response = request.post("#{path}/")
-
-          expect(response).to be_redirect
-          expect(subject).to disallow_request
-        end
-      end
-
-      where(:description, :path) do
-        'sign in route'     | '/users/sign_in'
-        'sign out route'    | '/users/sign_out'
-        'oauth token route' | '/oauth/token'
-      end
-
-      with_them do
-        it "expects a POST to #{description} URL to be allowed" do
-          response = request.post(path)
-
-          expect(response).not_to be_redirect
-          expect(subject).not_to disallow_request
-        end
-
-        it "expects a POST to #{description} URL with trailing slash to be allowed" do
-          response = request.post("#{path}/")
-
-          expect(response).not_to be_redirect
-          expect(subject).not_to disallow_request
-        end
-      end
-    end
   end
 end