Commit e5709c22 authored by Valery Sizov's avatar Valery Sizov

Allow grapqh requests to a secondary node

We only prohibit GraphQL mutation
parent d2c1eb44
......@@ -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
---
title: Fix GraphQL for read-only instances
merge_request: 17225
author:
type: fixed
......@@ -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
......
......@@ -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
......
......@@ -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
[
......
# 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
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment