Merge branch 'mk/fix-geo-node-status-when-maintenance-mode-is-on' into 'master'

Allow Geo node status updates during maintenance mode

See merge request gitlab-org/gitlab!70010

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