Merge branch '32279-fix_graphql_for_secondary_node' into 'master'

Allow GraphQL requests to a secondary node

Closes #32279

See merge request gitlab-org/gitlab!17225

app/graphql/mutations/base_mutation.rb

@@ -5,6 +5,8 @@ module Mutations
     prepend Gitlab::Graphql::Authorize::AuthorizeResource
     prepend Gitlab::Graphql::CopyFieldDescription
 
+    ERROR_MESSAGE = 'You cannot perform write operations on a read-only instance'
+
     field :errors, [GraphQL::STRING_TYPE],
           null: false,
           description: "Reasons why the mutation failed."
@@ -17,5 +19,13 @@ module Mutations
     def errors_on_object(record)
       record.errors.full_messages
     end
+
+    def ready?(**args)
+      if Gitlab::Database.read_only?
+        raise Gitlab::Graphql::Errors::ResourceNotAvailable, ERROR_MESSAGE
+      else
+        true
+      end
+    end
   end
 end

changelogs/unreleased/32279-fix_graphql_for_secondary_node.yml

+---
+title: Fix GraphQL for read-only instances
+merge_request: 17225
+author:
+type: fixed

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

@@ -20,10 +20,10 @@ module EE
 
           override :whitelisted_routes
           def whitelisted_routes
-            super || geo_node_update_route || geo_proxy_git_push_ssh_route
+            super || geo_node_update_route? || geo_proxy_git_push_ssh_route?
           end
 
-          def geo_node_update_route
+          def geo_node_update_route?
             # Calling route_hash may be expensive. Only do it if we think there's a possible match
             return false unless request.path.start_with?('/admin/geo/')
 
@@ -37,11 +37,12 @@ module EE
             end
           end
 
-          def geo_proxy_git_push_ssh_route
+          def geo_proxy_git_push_ssh_route?
             routes = ::Gitlab::Middleware::ReadOnly::API_VERSIONS.map do |version|
               %W(/api/v#{version}/geo/proxy_git_push_ssh/info_refs
                  /api/v#{version}/geo/proxy_git_push_ssh/push)
             end
+
             routes.flatten.include?(request.path)
           end
         end

lib/gitlab/middleware/read_only/controller.rb

@@ -20,6 +20,8 @@ module Gitlab
           'projects/lfs_locks_api' => %w{verify create unlock}
         }.freeze
 
+        GRAPHQL_URL = '/api/graphql'
+
         def initialize(app, env)
           @app = app
           @env = env
@@ -79,7 +81,7 @@ module Gitlab
 
         # Overridden in EE module
         def whitelisted_routes
-          grack_route? || internal_route? || lfs_route? || sidekiq_route?
+          grack_route? || internal_route? || lfs_route? || sidekiq_route? || graphql_query?
         end
 
         def grack_route?
@@ -108,6 +110,10 @@ module Gitlab
         def sidekiq_route?
           request.path.start_with?("#{relative_url}/admin/sidekiq")
         end
+
+        def graphql_query?
+          request.post? && request.path.start_with?(GRAPHQL_URL)
+        end
       end
     end
   end

spec/lib/gitlab/middleware/read_only_spec.rb

@@ -103,6 +103,13 @@ describe Gitlab::Middleware::ReadOnly do
         expect(subject).not_to disallow_request
       end
 
+      it 'expects a graphql request to be allowed' do
+        response = request.post("/api/graphql")
+
+        expect(response).not_to be_redirect
+        expect(subject).not_to disallow_request
+      end
+
       context 'sidekiq admin requests' do
         where(:mounted_at) do
           [

spec/requests/api/graphql/read_only_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe 'Requests on a read-only node' do
+  include GraphqlHelpers
+
+  before do
+    allow(Gitlab::Database).to receive(:read_only?) { true }
+  end
+
+  context 'mutations' do
+    let(:current_user) { note.author }
+    let!(:note) { create(:note) }
+
+    let(:mutation) do
+      variables = {
+        id: GitlabSchema.id_from_object(note).to_s
+      }
+
+      graphql_mutation(:destroy_note, variables)
+    end
+
+    def mutation_response
+      graphql_mutation_response(:destroy_note)
+    end
+
+    it 'disallows the query' do
+      post_graphql_mutation(mutation, current_user: current_user)
+
+      expect(json_response['errors'].first['message']).to eq(Mutations::BaseMutation::ERROR_MESSAGE)
+    end
+
+    it 'does not destroy the Note' do
+      expect do
+        post_graphql_mutation(mutation, current_user: current_user)
+      end.not_to change { Note.count }
+    end
+  end
+
+  context 'read-only queries' do
+    let(:current_user) { create(:user) }
+    let(:project) { create(:project, :repository) }
+
+    before do
+      project.add_developer(current_user)
+    end
+
+    it 'allows the query' do
+      query = graphql_query_for('project', 'fullPath' => project.full_path)
+
+      post_graphql(query, current_user: current_user)
+
+      expect(graphql_data['project']).not_to be_nil
+    end
+  end
+end