Automatic merge of gitlab-org/gitlab-ce master

app/graphql/gitlab_schema.rb

 # frozen_string_literal: true
 
 class GitlabSchema < GraphQL::Schema
+  # Took our current most complicated query in use, issues.graphql,
+  # with a complexity of 19, and added a 20 point buffer to it.
+  # These values will evolve over time.
+  DEFAULT_MAX_COMPLEXITY   = 40
+  AUTHENTICATED_COMPLEXITY = 50
+  ADMIN_COMPLEXITY         = 60
+
   use BatchLoader::GraphQL
   use Gitlab::Graphql::Authorize
   use Gitlab::Graphql::Present
   use Gitlab::Graphql::Connections
   use Gitlab::Graphql::Tracing
 
+  query_analyzer Gitlab::Graphql::QueryAnalyzers::LogQueryComplexity.analyzer
+
   query(Types::QueryType)
 
   default_max_page_size 100
+
+  max_complexity DEFAULT_MAX_COMPLEXITY
+
   mutation(Types::MutationType)
+
+  def self.execute(query_str = nil, **kwargs)
+    kwargs[:max_complexity] ||= max_query_complexity(kwargs[:context])
+
+    super(query_str, **kwargs)
+  end
+
+  def self.max_query_complexity(ctx)
+    current_user = ctx&.fetch(:current_user)
+
+    if current_user&.admin
+      ADMIN_COMPLEXITY
+    elsif current_user
+      AUTHENTICATED_COMPLEXITY
+    else
+      DEFAULT_MAX_COMPLEXITY
+    end
+  end
 end

app/graphql/types/base_field.rb

@@ -3,5 +3,14 @@
 module Types
   class BaseField < GraphQL::Schema::Field
     prepend Gitlab::Graphql::Authorize
+
+    DEFAULT_COMPLEXITY = 1
+
+    def initialize(*args, **kwargs, &block)
+      # complexity is already defaulted to 1, but let's make it explicit
+      kwargs[:complexity] ||= DEFAULT_COMPLEXITY
+
+      super(*args, **kwargs, &block)
+    end
   end
 end

changelogs/unreleased/58405-basic-limiting-complexity-of-graphql-queries.yml

+---
+title: Add initial complexity limits to GraphQL queries
+merge_request: 26629
+author:
+type: performance

changelogs/unreleased/58839-automatically-set-prometheus-step-interval.yml

+---
+title: Automatically set Prometheus step interval
+merge_request: 26441
+author:
+type: changed

lib/gitlab/graphql/query_analyzers/log_query_complexity.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Graphql
+    module QueryAnalyzers
+      class LogQueryComplexity
+        class << self
+          def analyzer
+            GraphQL::Analysis::QueryComplexity.new do |query, complexity|
+              # temporary until https://gitlab.com/gitlab-org/gitlab-ce/issues/59587
+              Rails.logger.info("[GraphQL Query Complexity] #{complexity}  | admin? #{query.context[:current_user]&.admin?}")
+            end
+          end
+        end
+      end
+    end
+  end
+end

lib/gitlab/prometheus_client.rb

@@ -6,6 +6,14 @@ module Gitlab
     Error = Class.new(StandardError)
     QueryError = Class.new(Gitlab::PrometheusClient::Error)
 
+    # Target number of data points for `query_range`.
+    # Please don't exceed the limit of 11000 data points
+    # See https://github.com/prometheus/prometheus/blob/91306bdf24f5395e2601773316945a478b4b263d/web/api/v1/api.go#L347
+    QUERY_RANGE_DATA_POINTS = 600
+
+    # Minimal value of the `step` parameter for `query_range` in seconds.
+    QUERY_RANGE_MIN_STEP = 60
+
     attr_reader :rest_client, :headers
 
     def initialize(rest_client)
@@ -23,12 +31,18 @@ module Gitlab
     end
 
     def query_range(query, start: 8.hours.ago, stop: Time.now)
+      start = start.to_f
+      stop = stop.to_f
+      step = self.class.compute_step(start, stop)
+
       get_result('matrix') do
-        json_api_get('query_range',
-                     query: query,
-                     start: start.to_f,
-                     end: stop.to_f,
-                     step: 1.minute.to_i)
+        json_api_get(
+          'query_range',
+          query: query,
+          start: start,
+          end: stop,
+          step: step
+        )
       end
     end
 
@@ -40,6 +54,14 @@ module Gitlab
       json_api_get('series', 'match': matches, start: start.to_f, end: stop.to_f)
     end
 
+    def self.compute_step(start, stop)
+      diff = stop - start
+
+      step = (diff / QUERY_RANGE_DATA_POINTS).ceil
+
+      [QUERY_RANGE_MIN_STEP, step].max
+    end
+
     private
 
     def json_api_get(type, args = {})

spec/graphql/gitlab_schema_spec.rb

+# frozen_string_literal: true
+
 require 'spec_helper'
 
 describe GitlabSchema do
@@ -31,6 +33,36 @@ describe GitlabSchema do
     expect(connection).to eq(Gitlab::Graphql::Connections::KeysetConnection)
   end
 
+  context 'for different types of users' do
+    it 'returns DEFAULT_MAX_COMPLEXITY for no user' do
+      expect(GraphQL::Schema).to receive(:execute).with('query', hash_including(max_complexity: GitlabSchema::DEFAULT_MAX_COMPLEXITY))
+
+      described_class.execute('query')
+    end
+
+    it 'returns AUTHENTICATED_COMPLEXITY for a logged in user' do
+      user = build :user
+
+      expect(GraphQL::Schema).to receive(:execute).with('query', hash_including(max_complexity: GitlabSchema::AUTHENTICATED_COMPLEXITY))
+
+      described_class.execute('query', context: { current_user: user })
+    end
+
+    it 'returns ADMIN_COMPLEXITY for an admin user' do
+      user = build :user, :admin
+
+      expect(GraphQL::Schema).to receive(:execute).with('query', hash_including(max_complexity: GitlabSchema::ADMIN_COMPLEXITY))
+
+      described_class.execute('query', context: { current_user: user })
+    end
+
+    it 'returns what was passed on the query' do
+      expect(GraphQL::Schema).to receive(:execute).with('query', { max_complexity: 1234 })
+
+      described_class.execute('query', max_complexity: 1234)
+    end
+  end
+
   def field_instrumenters
     described_class.instrumenters[:field]
   end

spec/graphql/types/base_field_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Types::BaseField do
+  context 'when considering complexity' do
+    it 'defaults to 1' do
+      field = described_class.new(name: 'test', type: GraphQL::STRING_TYPE, null: true)
+
+      expect(field.to_graphql.complexity).to eq 1
+    end
+
+    it 'has specified value' do
+      field = described_class.new(name: 'test', type: GraphQL::STRING_TYPE, null: true, complexity: 12)
+
+      expect(field.to_graphql.complexity).to eq 12
+    end
+  end
+end

spec/lib/gitlab/prometheus_client_spec.rb

@@ -230,4 +230,32 @@ describe Gitlab::PrometheusClient do
       let(:execute_query) { subject.query_range(prometheus_query) }
     end
   end
+
+  describe '.compute_step' do
+    using RSpec::Parameterized::TableSyntax
+
+    let(:now) { Time.now.utc }
+
+    subject { described_class.compute_step(start, stop) }
+
+    where(:time_interval_in_seconds, :step) do
+      0               | 60
+      10.hours        | 60
+      10.hours + 1    | 61
+      # frontend options
+      30.minutes      | 60
+      3.hours         | 60
+      8.hours         | 60
+      1.day           | 144
+      3.days          | 432
+      1.week          | 1008
+    end
+
+    with_them do
+      let(:start) { now - time_interval_in_seconds }
+      let(:stop) { now }
+
+      it { is_expected.to eq(step) }
+    end
+  end
 end

spec/requests/api/graphql/gitlab_schema_spec.rb

+require 'spec_helper'
+
+describe 'GitlabSchema configurations' do
+  include GraphqlHelpers
+
+  let(:project) { create(:project, :repository) }
+  let!(:query)  { graphql_query_for('project', 'fullPath' => project.full_path) }
+
+  it 'shows an error if complexity it too high' do
+    allow(GitlabSchema).to receive(:max_query_complexity).and_return 1
+
+    post_graphql(query, current_user: nil)
+
+    expect(graphql_errors.first['message']).to include('which exceeds max complexity of 1')
+  end
+end

spec/support/helpers/graphql_helpers.rb

@@ -93,6 +93,8 @@ module GraphqlHelpers
   end
 
   def all_graphql_fields_for(class_name, parent_types = Set.new)
+    allow_unlimited_graphql_complexity
+
     type = GitlabSchema.types[class_name.to_s]
     return "" unless type
 
@@ -170,4 +172,10 @@ module GraphqlHelpers
 
     field_type
   end
+
+  # for most tests, we want to allow unlimited complexity
+  def allow_unlimited_graphql_complexity
+    allow_any_instance_of(GitlabSchema).to receive(:max_complexity).and_return nil
+    allow(GitlabSchema).to receive(:max_query_complexity).with(any_args).and_return nil
+  end
 end

spec/support/helpers/prometheus_helpers.rb

@@ -25,12 +25,16 @@ module PrometheusHelpers
     "https://prometheus.example.com/api/v1/query?#{query}"
   end
 
-  def prometheus_query_range_url(prometheus_query, start: 8.hours.ago, stop: Time.now.to_f)
+  def prometheus_query_range_url(prometheus_query, start: 8.hours.ago, stop: Time.now, step: nil)
+    start = start.to_f
+    stop = stop.to_f
+    step ||= Gitlab::PrometheusClient.compute_step(start, stop)
+
     query = {
       query: prometheus_query,
-      start: start.to_f,
+      start: start,
       end: stop,
-      step: 1.minute.to_i
+      step: step
     }.to_query
 
     "https://prometheus.example.com/api/v1/query_range?#{query}"