diff options
-rw-r--r-- | app/graphql/mutations/base_mutation.rb | 10 | ||||
-rw-r--r-- | changelogs/unreleased/32279-fix_graphql_for_secondary_node.yml | 5 | ||||
-rw-r--r-- | lib/gitlab/middleware/read_only/controller.rb | 8 | ||||
-rw-r--r-- | locale/gitlab.pot | 6 | ||||
-rw-r--r-- | spec/lib/gitlab/middleware/read_only_spec.rb | 7 | ||||
-rw-r--r-- | spec/requests/api/graphql/read_only_spec.rb | 57 |
6 files changed, 92 insertions, 1 deletions
diff --git a/app/graphql/mutations/base_mutation.rb b/app/graphql/mutations/base_mutation.rb index 7273a74cb86..623f7c27584 100644 --- a/app/graphql/mutations/base_mutation.rb +++ b/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 diff --git a/changelogs/unreleased/32279-fix_graphql_for_secondary_node.yml b/changelogs/unreleased/32279-fix_graphql_for_secondary_node.yml new file mode 100644 index 00000000000..746ed2e2333 --- /dev/null +++ b/changelogs/unreleased/32279-fix_graphql_for_secondary_node.yml @@ -0,0 +1,5 @@ +--- +title: Fix GraphQL for read-only instances +merge_request: 17225 +author: +type: fixed diff --git a/lib/gitlab/middleware/read_only/controller.rb b/lib/gitlab/middleware/read_only/controller.rb index a29dc5395f3..907e031a02e 100644 --- a/lib/gitlab/middleware/read_only/controller.rb +++ b/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 diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 9be6cbca00c..089a6dd5b1d 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -9705,6 +9705,12 @@ msgstr "" msgid "Metric was successfully updated." msgstr "" +msgid "MetricChart|Please select a metric" +msgstr "" + +msgid "MetricChart|Selected" +msgstr "" + msgid "Metrics" msgstr "" diff --git a/spec/lib/gitlab/middleware/read_only_spec.rb b/spec/lib/gitlab/middleware/read_only_spec.rb index d2c8f4ab0bd..c7e9b38e3ca 100644 --- a/spec/lib/gitlab/middleware/read_only_spec.rb +++ b/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 [ diff --git a/spec/requests/api/graphql/read_only_spec.rb b/spec/requests/api/graphql/read_only_spec.rb new file mode 100644 index 00000000000..1d28a71258d --- /dev/null +++ b/spec/requests/api/graphql/read_only_spec.rb @@ -0,0 +1,57 @@ +# 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 |