diff options
28 files changed, 855 insertions, 21 deletions
diff --git a/app/controllers/concerns/render_service_results.rb b/app/controllers/concerns/render_service_results.rb new file mode 100644 index 00000000000..0149a71d9f5 --- /dev/null +++ b/app/controllers/concerns/render_service_results.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +module RenderServiceResults + extend ActiveSupport::Concern + + def success_response(result) + render({ + status: result[:http_status], + json: result[:body] + }) + end + + def continue_polling_response + render({ + status: :no_content, + json: { + status: _('processing'), + message: _('Not ready yet. Try again later.') + } + }) + end + + def error_response(result) + render({ + status: result[:http_status] || :bad_request, + json: { status: result[:status], message: result[:message] } + }) + end +end diff --git a/app/controllers/projects/environments/prometheus_api_controller.rb b/app/controllers/projects/environments/prometheus_api_controller.rb index 9c6c6513a78..e902d218c75 100644 --- a/app/controllers/projects/environments/prometheus_api_controller.rb +++ b/app/controllers/projects/environments/prometheus_api_controller.rb @@ -1,6 +1,8 @@ # frozen_string_literal: true class Projects::Environments::PrometheusApiController < Projects::ApplicationController + include RenderServiceResults + before_action :authorize_read_prometheus! before_action :environment @@ -12,21 +14,10 @@ class Projects::Environments::PrometheusApiController < Projects::ApplicationCon proxy_params ).execute - if result.nil? - return render status: :no_content, json: { - status: _('processing'), - message: _('Not ready yet. Try again later.') - } - end - - if result[:status] == :success - render status: result[:http_status], json: result[:body] - else - render( - status: result[:http_status] || :bad_request, - json: { status: result[:status], message: result[:message] } - ) - end + return continue_polling_response if result.nil? + return error_response(result) if result[:status] == :error + + success_response(result) end private diff --git a/app/controllers/projects/grafana_api_controller.rb b/app/controllers/projects/grafana_api_controller.rb new file mode 100644 index 00000000000..4bdf4c12cac --- /dev/null +++ b/app/controllers/projects/grafana_api_controller.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +class Projects::GrafanaApiController < Projects::ApplicationController + include RenderServiceResults + + def proxy + result = ::Grafana::ProxyService.new( + project, + params[:datasource_id], + params[:proxy_path], + query_params.to_h + ).execute + + return continue_polling_response if result.nil? + return error_response(result) if result[:status] == :error + + success_response(result) + end + + private + + def query_params + params.permit(:query, :start, :end, :step) + end +end diff --git a/app/controllers/projects/merge_requests/diffs_controller.rb b/app/controllers/projects/merge_requests/diffs_controller.rb index 7b694dcdd77..1913d7cd580 100644 --- a/app/controllers/projects/merge_requests/diffs_controller.rb +++ b/app/controllers/projects/merge_requests/diffs_controller.rb @@ -7,7 +7,7 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic before_action :apply_diff_view_cookie! before_action :commit, except: :diffs_batch before_action :define_diff_vars, except: :diffs_batch - before_action :define_diff_comment_vars, except: :diffs_batch + before_action :define_diff_comment_vars, except: [:diffs_batch, :diffs_metadata] def show render_diffs @@ -37,6 +37,11 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic render json: PaginatedDiffSerializer.new(current_user: current_user).represent(diffs, options) end + def diffs_metadata + render json: DiffsMetadataSerializer.new(project: @merge_request.project) + .represent(@diffs, additional_attributes) + end + private def preloadable_mr_relations diff --git a/app/helpers/diff_helper.rb b/app/helpers/diff_helper.rb index 6aafd856423..52ec2eadf5e 100644 --- a/app/helpers/diff_helper.rb +++ b/app/helpers/diff_helper.rb @@ -203,8 +203,8 @@ module DiffHelper link_to "#{hide_whitespace? ? 'Show' : 'Hide'} whitespace changes", url, class: options[:class] end - def render_overflow_warning?(diff_files) - diffs = @merge_request_diff.presence || diff_files + def render_overflow_warning?(diffs_collection) + diffs = @merge_request_diff.presence || diffs_collection.diff_files diffs.overflow? end diff --git a/app/models/grafana_integration.rb b/app/models/grafana_integration.rb index 668b9dafd7d..51cc398394d 100644 --- a/app/models/grafana_integration.rb +++ b/app/models/grafana_integration.rb @@ -13,4 +13,8 @@ class GrafanaIntegration < ApplicationRecord addressable_url: { enforce_sanitization: true, ascii_only: true } validates :token, :project, presence: true + + def client + @client ||= ::Grafana::Client.new(api_url: grafana_url.chomp('/'), token: token) + end end diff --git a/app/models/repository_language.rb b/app/models/repository_language.rb index e6867f905e2..e468d716239 100644 --- a/app/models/repository_language.rb +++ b/app/models/repository_language.rb @@ -7,7 +7,7 @@ class RepositoryLanguage < ApplicationRecord default_scope { includes(:programming_language) } validates :project, presence: true - validates :share, inclusion: { in: 0..100, message: "The share of a lanuage is between 0 and 100" } + validates :share, inclusion: { in: 0..100, message: "The share of a language is between 0 and 100" } validates :programming_language, uniqueness: { scope: :project_id } delegate :name, :color, to: :programming_language diff --git a/app/serializers/diff_file_metadata_entity.rb b/app/serializers/diff_file_metadata_entity.rb new file mode 100644 index 00000000000..500a844b170 --- /dev/null +++ b/app/serializers/diff_file_metadata_entity.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +class DiffFileMetadataEntity < Grape::Entity + expose :added_lines + expose :removed_lines + expose :new_path + expose :old_path + expose :new_file?, as: :new_file + expose :deleted_file?, as: :deleted_file +end diff --git a/app/serializers/diffs_entity.rb b/app/serializers/diffs_entity.rb index 1763fe5b6ab..19875a1287c 100644 --- a/app/serializers/diffs_entity.rb +++ b/app/serializers/diffs_entity.rb @@ -53,7 +53,7 @@ class DiffsEntity < Grape::Entity # rubocop: enable CodeReuse/ActiveRecord expose :render_overflow_warning do |diffs| - render_overflow_warning?(diffs.diff_files) + render_overflow_warning?(diffs) end expose :email_patch_path, if: -> (*) { merge_request } do |diffs| diff --git a/app/serializers/diffs_metadata_entity.rb b/app/serializers/diffs_metadata_entity.rb new file mode 100644 index 00000000000..c82c686e8ef --- /dev/null +++ b/app/serializers/diffs_metadata_entity.rb @@ -0,0 +1,6 @@ +# frozen_string_literal: true + +class DiffsMetadataEntity < DiffsEntity + unexpose :diff_files + expose :diff_files, using: DiffFileMetadataEntity +end diff --git a/app/serializers/diffs_metadata_serializer.rb b/app/serializers/diffs_metadata_serializer.rb new file mode 100644 index 00000000000..9f3ba7b778d --- /dev/null +++ b/app/serializers/diffs_metadata_serializer.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +class DiffsMetadataSerializer < BaseSerializer + entity DiffsMetadataEntity +end diff --git a/app/services/grafana/proxy_service.rb b/app/services/grafana/proxy_service.rb new file mode 100644 index 00000000000..74fcdc750b0 --- /dev/null +++ b/app/services/grafana/proxy_service.rb @@ -0,0 +1,83 @@ +# frozen_string_literal: true + +# Proxies calls to a Grafana-integrated Prometheus instance +# through the Grafana proxy API + +# This allows us to fetch and render metrics in GitLab from a Prometheus +# instance for which dashboards are configured in Grafana +module Grafana + class ProxyService < BaseService + include ReactiveCaching + + self.reactive_cache_key = ->(service) { service.cache_key } + self.reactive_cache_lease_timeout = 30.seconds + self.reactive_cache_refresh_interval = 30.seconds + self.reactive_cache_worker_finder = ->(_id, *args) { from_cache(*args) } + + attr_accessor :project, :datasource_id, :proxy_path, :query_params + + # @param project_id [Integer] Project id for which grafana is configured. + # + # See #initialize for other parameters. + def self.from_cache(project_id, datasource_id, proxy_path, query_params) + project = Project.find(project_id) + + new(project, datasource_id, proxy_path, query_params) + end + + # @param project [Project] Project for which grafana is configured. + # @param datasource_id [String] Grafana datasource id for Prometheus instance + # @param proxy_path [String] Path to Prometheus endpoint; EX) 'api/v1/query_range' + # @param query_params [Hash<String, String>] Supported params: [query, start, end, step] + def initialize(project, datasource_id, proxy_path, query_params) + @project = project + @datasource_id = datasource_id + @proxy_path = proxy_path + @query_params = query_params + end + + def execute + return cannot_proxy_response unless client + + with_reactive_cache(*cache_key) { |result| result } + end + + def calculate_reactive_cache(*) + return cannot_proxy_response unless client + + response = client.proxy_datasource( + datasource_id: datasource_id, + proxy_path: proxy_path, + query: query_params + ) + + success(http_status: response.code, body: response.body) + rescue ::Grafana::Client::Error => error + service_unavailable_response(error) + end + + # Required for ReactiveCaching; Usage overridden by + # self.reactive_cache_worker_finder + def id + nil + end + + def cache_key + [project.id, datasource_id, proxy_path, query_params] + end + + private + + def client + project.grafana_integration&.client + end + + def service_unavailable_response(exception) + error(exception.message, :service_unavailable) + end + + def cannot_proxy_response + error('Proxy support for this API is not available currently') + end + end +end diff --git a/app/views/projects/diffs/_diffs.html.haml b/app/views/projects/diffs/_diffs.html.haml index 2dba3fcd664..cf7fe36af9d 100644 --- a/app/views/projects/diffs/_diffs.html.haml +++ b/app/views/projects/diffs/_diffs.html.haml @@ -21,7 +21,7 @@ = parallel_diff_btn = render 'projects/diffs/stats', diff_files: diff_files -- if render_overflow_warning?(diff_files) +- if render_overflow_warning?(diffs) = render 'projects/diffs/warning', diff_files: diffs .files{ data: { can_create_note: can_create_note } } diff --git a/changelogs/unreleased/28781-migrate-pages-metadata-in-background.yml b/changelogs/unreleased/28781-migrate-pages-metadata-in-background.yml new file mode 100644 index 00000000000..171fd8ff554 --- /dev/null +++ b/changelogs/unreleased/28781-migrate-pages-metadata-in-background.yml @@ -0,0 +1,5 @@ +--- +title: Schedule background migration to populate pages metadata +merge_request: 17993 +author: +type: added diff --git a/changelogs/unreleased/feature-default-cluster-to-vpc-enabled.yml b/changelogs/unreleased/feature-default-cluster-to-vpc-enabled.yml new file mode 100644 index 00000000000..feb000554a8 --- /dev/null +++ b/changelogs/unreleased/feature-default-cluster-to-vpc-enabled.yml @@ -0,0 +1,5 @@ +--- +title: Create clusters with VPC-Native enabled +merge_request: 18284 +author: +type: changed diff --git a/changelogs/unreleased/osw-diffs-metadata-endpoint.yml b/changelogs/unreleased/osw-diffs-metadata-endpoint.yml new file mode 100644 index 00000000000..c8da00249f1 --- /dev/null +++ b/changelogs/unreleased/osw-diffs-metadata-endpoint.yml @@ -0,0 +1,5 @@ +--- +title: Introduce a lightweight diffs_metadata endpoint +merge_request: 18104 +author: +type: added diff --git a/changelogs/unreleased/sy-grafana-proxy.yml b/changelogs/unreleased/sy-grafana-proxy.yml new file mode 100644 index 00000000000..2c2a3959bff --- /dev/null +++ b/changelogs/unreleased/sy-grafana-proxy.yml @@ -0,0 +1,5 @@ +--- +title: Add endpoint to proxy requests to grafana's proxy endpoint +merge_request: 18210 +author: +type: added diff --git a/config/routes/project.rb b/config/routes/project.rb index f9331a22bc8..7d51cfd6dee 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -186,6 +186,10 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do resource :import, only: [:new, :create, :show] resource :avatar, only: [:show, :destroy] + + get 'grafana/proxy/:datasource_id/*proxy_path', + to: 'grafana_api#proxy', + as: :grafana_api end # End of the /-/ scope. @@ -282,6 +286,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do get :pipelines get :diffs, to: 'merge_requests/diffs#show' get :diffs_batch, to: 'merge_requests/diffs#diffs_batch' + get :diffs_metadata, to: 'merge_requests/diffs#diffs_metadata' get :widget, to: 'merge_requests/content#widget' get :cached_widget, to: 'merge_requests/content#cached_widget' end diff --git a/db/post_migrate/20191002031332_schedule_pages_metadata_migration.rb b/db/post_migrate/20191002031332_schedule_pages_metadata_migration.rb new file mode 100644 index 00000000000..0cd24da50d0 --- /dev/null +++ b/db/post_migrate/20191002031332_schedule_pages_metadata_migration.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +class SchedulePagesMetadataMigration < ActiveRecord::Migration[5.2] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + BATCH_SIZE = 10_000 + MIGRATION = 'MigratePagesMetadata' + + disable_ddl_transaction! + + class Project < ActiveRecord::Base + include ::EachBatch + + self.table_name = 'projects' + end + + def up + say "Scheduling `#{MIGRATION}` jobs" + + # At the time of writing there are ~10_669_292 records to be inserted for GitLab.com, + # batches of 10_000 with delay interval of 2 minutes gives us an estimate of close to 36 hours. + queue_background_migration_jobs_by_range_at_intervals(Project, MIGRATION, 2.minutes, batch_size: BATCH_SIZE) + end + + def down + # no-op + end +end diff --git a/lib/google_api/cloud_platform/client.rb b/lib/google_api/cloud_platform/client.rb index eaf94708282..9085835dee6 100644 --- a/lib/google_api/cloud_platform/client.rb +++ b/lib/google_api/cloud_platform/client.rb @@ -96,6 +96,9 @@ module GoogleApi legacy_abac: { enabled: legacy_abac }, + ip_allocation_policy: { + use_ip_aliases: true + }, addons_config: enable_addons.each_with_object({}) do |addon, hash| hash[addon] = { disabled: false } end diff --git a/lib/grafana/client.rb b/lib/grafana/client.rb new file mode 100644 index 00000000000..0765630f9bb --- /dev/null +++ b/lib/grafana/client.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +module Grafana + class Client + Error = Class.new(StandardError) + + # @param api_url [String] Base URL of the Grafana instance + # @param token [String] Admin-level API token for instance + def initialize(api_url:, token:) + @api_url = api_url + @token = token + end + + # @param datasource_id [String] Grafana ID for the datasource + # @param proxy_path [String] Path to proxy - ex) 'api/v1/query_range' + def proxy_datasource(datasource_id:, proxy_path:, query: {}) + http_get("#{@api_url}/api/datasources/proxy/#{datasource_id}/#{proxy_path}", query: query) + end + + private + + def http_get(url, params = {}) + response = handle_request_exceptions do + Gitlab::HTTP.get(url, **request_params.merge(params)) + end + + handle_response(response) + end + + def request_params + { + headers: { + 'Authorization' => "Bearer #{@token}", + 'Accept' => 'application/json', + 'Content-Type' => 'application/json' + }, + follow_redirects: false + } + end + + def handle_request_exceptions + yield + rescue Gitlab::HTTP::Error + raise_error 'Error when connecting to Grafana' + rescue Net::OpenTimeout + raise_error 'Connection to Grafana timed out' + rescue SocketError + raise_error 'Received SocketError when trying to connect to Grafana' + rescue OpenSSL::SSL::SSLError + raise_error 'Grafana returned invalid SSL data' + rescue Errno::ECONNREFUSED + raise_error 'Connection refused' + rescue => e + raise_error "Grafana request failed due to #{e.class}" + end + + def handle_response(response) + return response if response.code == 200 + + raise_error "Grafana response status code: #{response.code}" + end + + def raise_error(message) + raise Client::Error, message + end + end +end diff --git a/spec/controllers/projects/grafana_api_controller_spec.rb b/spec/controllers/projects/grafana_api_controller_spec.rb new file mode 100644 index 00000000000..352a364295b --- /dev/null +++ b/spec/controllers/projects/grafana_api_controller_spec.rb @@ -0,0 +1,97 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Projects::GrafanaApiController do + let_it_be(:project) { create(:project) } + let_it_be(:user) { create(:user) } + + before do + project.add_reporter(user) + sign_in(user) + end + + describe 'GET #proxy' do + let(:proxy_service) { instance_double(Grafana::ProxyService) } + let(:params) do + { + namespace_id: project.namespace.full_path, + project_id: project.name, + proxy_path: 'api/v1/query_range', + datasource_id: '1', + query: 'rate(relevant_metric)', + start: '1570441248', + end: '1570444848', + step: '900' + } + end + + before do + allow(Grafana::ProxyService).to receive(:new).and_return(proxy_service) + allow(proxy_service).to receive(:execute).and_return(service_result) + end + + shared_examples_for 'error response' do |http_status| + it "returns #{http_status}" do + get :proxy, params: params + + expect(response).to have_gitlab_http_status(http_status) + expect(json_response['status']).to eq('error') + expect(json_response['message']).to eq('error message') + end + end + + context 'with a successful result' do + let(:service_result) { { status: :success, body: '{}' } } + + it 'returns a grafana datasource response' do + get :proxy, params: params + + expect(Grafana::ProxyService) + .to have_received(:new) + .with(project, '1', 'api/v1/query_range', + params.slice(:query, :start, :end, :step).stringify_keys) + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response).to eq({}) + end + end + + context 'when the request is still unavailable' do + let(:service_result) { nil } + + it 'returns 204 no content' do + get :proxy, params: params + + expect(response).to have_gitlab_http_status(:no_content) + expect(json_response['status']).to eq('processing') + expect(json_response['message']).to eq('Not ready yet. Try again later.') + end + end + + context 'when an error has occurred' do + context 'with an error accessing grafana' do + let(:service_result) do + { + http_status: :service_unavailable, + status: :error, + message: 'error message' + } + end + + it_behaves_like 'error response', :service_unavailable + end + + context 'with a processing error' do + let(:service_result) do + { + status: :error, + message: 'error message' + } + end + + it_behaves_like 'error response', :bad_request + end + end + end +end diff --git a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb index 302de3246c2..e677e836145 100644 --- a/spec/controllers/projects/merge_requests/diffs_controller_spec.rb +++ b/spec/controllers/projects/merge_requests/diffs_controller_spec.rb @@ -100,6 +100,136 @@ describe Projects::MergeRequests::DiffsController do it_behaves_like 'persisted preferred diff view cookie' end + describe 'GET diffs_metadata' do + def go(extra_params = {}) + params = { + namespace_id: project.namespace.to_param, + project_id: project, + id: merge_request.iid, + format: 'json' + } + + get :diffs_metadata, params: params.merge(extra_params) + end + + context 'when not authorized' do + let(:another_user) { create(:user) } + + before do + sign_in(another_user) + end + + it 'returns 404 when not a member' do + go + + expect(response).to have_gitlab_http_status(404) + end + + it 'returns 404 when visibility level is not enough' do + project.add_guest(another_user) + + go + + expect(response).to have_gitlab_http_status(404) + end + end + + context 'when diffable does not exists' do + it 'returns 404' do + go(diff_id: 9999) + + expect(response).to have_gitlab_http_status(404) + end + end + + context 'with valid diff_id' do + it 'returns success' do + go(diff_id: merge_request.merge_request_diff.id) + + expect(response).to have_gitlab_http_status(200) + end + + it 'serializes diffs metadata with expected arguments' do + expected_options = { + environment: nil, + merge_request: merge_request, + merge_request_diff: merge_request.merge_request_diff, + merge_request_diffs: merge_request.merge_request_diffs, + start_version: nil, + start_sha: nil, + commit: nil, + latest_diff: true + } + + expect_next_instance_of(DiffsMetadataSerializer) do |instance| + expect(instance).to receive(:represent) + .with(an_instance_of(Gitlab::Diff::FileCollection::MergeRequestDiff), expected_options) + .and_call_original + end + + go(diff_id: merge_request.merge_request_diff.id) + end + end + + context 'with MR regular diff params' do + it 'returns success' do + go + + expect(response).to have_gitlab_http_status(200) + end + + it 'serializes diffs metadata with expected arguments' do + expected_options = { + environment: nil, + merge_request: merge_request, + merge_request_diff: merge_request.merge_request_diff, + merge_request_diffs: merge_request.merge_request_diffs, + start_version: nil, + start_sha: nil, + commit: nil, + latest_diff: true + } + + expect_next_instance_of(DiffsMetadataSerializer) do |instance| + expect(instance).to receive(:represent) + .with(an_instance_of(Gitlab::Diff::FileCollection::MergeRequestDiff), expected_options) + .and_call_original + end + + go + end + end + + context 'with commit param' do + it 'returns success' do + go(commit_id: merge_request.diff_head_sha) + + expect(response).to have_gitlab_http_status(200) + end + + it 'serializes diffs metadata with expected arguments' do + expected_options = { + environment: nil, + merge_request: merge_request, + merge_request_diff: nil, + merge_request_diffs: merge_request.merge_request_diffs, + start_version: nil, + start_sha: nil, + commit: merge_request.diff_head_commit, + latest_diff: nil + } + + expect_next_instance_of(DiffsMetadataSerializer) do |instance| + expect(instance).to receive(:represent) + .with(an_instance_of(Gitlab::Diff::FileCollection::Commit), expected_options) + .and_call_original + end + + go(commit_id: merge_request.diff_head_sha) + end + end + end + describe 'GET diff_for_path' do def diff_for_path(extra_params = {}) params = { diff --git a/spec/lib/google_api/cloud_platform/client_spec.rb b/spec/lib/google_api/cloud_platform/client_spec.rb index 2253feb376d..91b076c31d6 100644 --- a/spec/lib/google_api/cloud_platform/client_spec.rb +++ b/spec/lib/google_api/cloud_platform/client_spec.rb @@ -102,6 +102,9 @@ describe GoogleApi::CloudPlatform::Client do legacy_abac: { enabled: legacy_abac }, + ip_allocation_policy: { + use_ip_aliases: true + }, addons_config: addons_config } } diff --git a/spec/lib/grafana/client_spec.rb b/spec/lib/grafana/client_spec.rb new file mode 100644 index 00000000000..bd93a3c59a2 --- /dev/null +++ b/spec/lib/grafana/client_spec.rb @@ -0,0 +1,107 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Grafana::Client do + let(:grafana_url) { 'https://grafanatest.com/-/grafana-project' } + let(:token) { 'test-token' } + + subject(:client) { described_class.new(api_url: grafana_url, token: token) } + + shared_examples 'calls grafana api' do + let!(:grafana_api_request) { stub_grafana_request(grafana_api_url) } + + it 'calls grafana api' do + subject + + expect(grafana_api_request).to have_been_requested + end + end + + shared_examples 'no redirects' do + let(:redirect_to) { 'https://redirected.example.com' } + let(:other_url) { 'https://grafana.example.org' } + + let!(:redirected_req_stub) { stub_grafana_request(other_url) } + + let!(:redirect_req_stub) do + stub_grafana_request( + grafana_api_url, + status: 302, + headers: { location: redirect_to } + ) + end + + it 'does not follow redirects' do + expect { subject }.to raise_exception( + Grafana::Client::Error, + 'Grafana response status code: 302' + ) + + expect(redirect_req_stub).to have_been_requested + expect(redirected_req_stub).not_to have_been_requested + end + end + + shared_examples 'handles exceptions' do + exceptions = { + Gitlab::HTTP::Error => 'Error when connecting to Grafana', + Net::OpenTimeout => 'Connection to Grafana timed out', + SocketError => 'Received SocketError when trying to connect to Grafana', + OpenSSL::SSL::SSLError => 'Grafana returned invalid SSL data', + Errno::ECONNREFUSED => 'Connection refused', + StandardError => 'Grafana request failed due to StandardError' + } + + exceptions.each do |exception, message| + context "#{exception}" do + before do + stub_request(:get, grafana_api_url).to_raise(exception) + end + + it do + expect { subject } + .to raise_exception(Grafana::Client::Error, message) + end + end + end + end + + describe '#proxy_datasource' do + let(:grafana_api_url) do + 'https://grafanatest.com/-/grafana-project/' \ + 'api/datasources/proxy/' \ + '1/api/v1/query_range' \ + '?query=rate(relevant_metric)' \ + '&start=1570441248&end=1570444848&step=900' + end + + subject do + client.proxy_datasource( + datasource_id: '1', + proxy_path: 'api/v1/query_range', + query: { + query: 'rate(relevant_metric)', + start: 1570441248, + end: 1570444848, + step: 900 + } + ) + end + + it_behaves_like 'calls grafana api' + it_behaves_like 'no redirects' + it_behaves_like 'handles exceptions' + end + + private + + def stub_grafana_request(url, body: {}, status: 200, headers: {}) + stub_request(:get, url) + .to_return( + status: status, + headers: { 'Content-Type' => 'application/json' }.merge(headers), + body: body.to_json + ) + end +end diff --git a/spec/migrations/schedule_pages_metadata_migration_spec.rb b/spec/migrations/schedule_pages_metadata_migration_spec.rb new file mode 100644 index 00000000000..100ed520a32 --- /dev/null +++ b/spec/migrations/schedule_pages_metadata_migration_spec.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +require 'spec_helper' +require Rails.root.join('db', 'post_migrate', '20191002031332_schedule_pages_metadata_migration') + +describe SchedulePagesMetadataMigration, :migration, :sidekiq do + let(:namespaces) { table(:namespaces) } + let(:projects) { table(:projects) } + + before do + stub_const("#{described_class.name}::BATCH_SIZE", 1) + + namespaces.create!(id: 11, name: 'gitlab', path: 'gitlab-org') + projects.create!(id: 111, namespace_id: 11, name: 'Project 111') + projects.create!(id: 114, namespace_id: 11, name: 'Project 114') + end + + it 'schedules pages metadata migration' do + Sidekiq::Testing.fake! do + Timecop.freeze do + migrate! + + expect(described_class::MIGRATION).to be_scheduled_delayed_migration(2.minutes, 111, 111) + expect(described_class::MIGRATION).to be_scheduled_delayed_migration(4.minutes, 114, 114) + expect(BackgroundMigrationWorker.jobs.size).to eq(2) + end + end + end +end diff --git a/spec/serializers/diffs_metadata_entity_spec.rb b/spec/serializers/diffs_metadata_entity_spec.rb new file mode 100644 index 00000000000..aaca393ec27 --- /dev/null +++ b/spec/serializers/diffs_metadata_entity_spec.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe DiffsMetadataEntity do + let(:user) { create(:user) } + let(:project) { create(:project, :repository) } + let(:request) { EntityRequest.new(project: project, current_user: user) } + let(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: project) } + let(:merge_request_diffs) { merge_request.merge_request_diffs } + let(:merge_request_diff) { merge_request_diffs.last } + + let(:entity) do + described_class.new(merge_request_diff.diffs, + request: request, + merge_request: merge_request, + merge_request_diffs: merge_request_diffs) + end + + context 'as json' do + subject { entity.as_json } + + it 'contain only required attributes' do + expect(subject.keys).to contain_exactly( + # Inherited attributes + :real_size, :size, :branch_name, + :target_branch_name, :commit, :merge_request_diff, + :start_version, :latest_diff, :latest_version_path, + :added_lines, :removed_lines, :render_overflow_warning, + :email_patch_path, :plain_diff_path, + :merge_request_diffs, + # Attributes + :diff_files + ) + end + + describe 'diff_files' do + it 'returns diff files metadata' do + payload = + DiffFileMetadataEntity.represent(merge_request_diff.diffs.diff_files).as_json + + expect(subject[:diff_files]).to eq(payload) + end + end + end +end diff --git a/spec/services/grafana/proxy_service_spec.rb b/spec/services/grafana/proxy_service_spec.rb new file mode 100644 index 00000000000..694d531c9fc --- /dev/null +++ b/spec/services/grafana/proxy_service_spec.rb @@ -0,0 +1,139 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Grafana::ProxyService do + include ReactiveCachingHelpers + + let_it_be(:project) { create(:project) } + let_it_be(:grafana_integration) { create(:grafana_integration, project: project) } + + let(:proxy_path) { 'api/v1/query_range' } + let(:datasource_id) { '1' } + let(:query_params) do + { + 'query' => 'rate(relevant_metric)', + 'start' => '1570441248', + 'end' => '1570444848', + 'step' => '900' + } + end + + let(:cache_params) { [project.id, datasource_id, proxy_path, query_params] } + + let(:service) do + described_class.new(project, datasource_id, proxy_path, query_params) + end + + shared_examples_for 'initializes an instance' do + it 'initializes an instance of ProxyService class' do + expect(subject).to be_an_instance_of(described_class) + expect(subject.project).to eq(project) + expect(subject.datasource_id).to eq('1') + expect(subject.proxy_path).to eq('api/v1/query_range') + expect(subject.query_params).to eq(query_params) + end + end + + describe '.from_cache' do + subject { described_class.from_cache(*cache_params) } + + it_behaves_like 'initializes an instance' + end + + describe '#initialize' do + subject { service } + + it_behaves_like 'initializes an instance' + end + + describe '#execute' do + subject(:result) { service.execute } + + context 'when grafana integration is not configured' do + before do + allow(project).to receive(:grafana_integration).and_return(nil) + end + + it 'returns error' do + expect(result).to eq( + status: :error, + message: 'Proxy support for this API is not available currently' + ) + end + end + + context 'with caching', :use_clean_rails_memory_store_caching do + context 'when value not present in cache' do + it 'returns nil' do + expect(ReactiveCachingWorker) + .to receive(:perform_async) + .with(service.class, service.id, *cache_params) + + expect(result).to eq(nil) + end + end + + context 'when value present in cache' do + let(:return_value) { { 'http_status' => 200, 'body' => 'body' } } + + before do + stub_reactive_cache(service, return_value, cache_params) + end + + it 'returns cached value' do + expect(ReactiveCachingWorker) + .not_to receive(:perform_async) + .with(service.class, service.id, *cache_params) + + expect(result[:http_status]).to eq(return_value[:http_status]) + expect(result[:body]).to eq(return_value[:body]) + end + end + end + + context 'call prometheus api' do + let(:client) { service.send(:client) } + + before do + synchronous_reactive_cache(service) + end + + context 'connection to grafana datasource succeeds' do + let(:response) { instance_double(Gitlab::HTTP::Response) } + let(:status_code) { 400 } + let(:body) { 'body' } + + before do + allow(client).to receive(:proxy_datasource).and_return(response) + + allow(response).to receive(:code).and_return(status_code) + allow(response).to receive(:body).and_return(body) + end + + it 'returns the http status code and body from prometheus' do + expect(result).to eq( + http_status: status_code, + body: body, + status: :success + ) + end + end + + context 'connection to grafana datasource fails' do + before do + allow(client).to receive(:proxy_datasource) + .and_raise(Grafana::Client::Error, 'Network connection error') + end + + it 'returns error' do + expect(result).to eq( + status: :error, + message: 'Network connection error', + http_status: :service_unavailable + ) + end + end + end + end +end |