From 8e28b42532fa7c0c6f33113187d7378a1e48a1c6 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Thu, 26 Nov 2020 21:09:22 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- .../javascripts/lib/utils/common_utils.js | 22 ++- app/controllers/concerns/service_params.rb | 3 + app/models/project.rb | 3 + .../project_services/datadog_service.rb | 124 ++++++++++++ .../pipelines_email_service.rb | 4 + app/models/service.rb | 10 +- .../integrations/test/project_service.rb | 6 +- .../defect-resolve-all-scroll-height.yml | 5 + .../development/datadog_ci_integration.yml | 7 + .../structure_load_in_transaction.rb | 9 + .../graphql/reference/gitlab_schema.graphql | 1 + doc/api/graphql/reference/gitlab_schema.json | 6 + doc/api/graphql/reference/index.md | 1 + lib/api/helpers/services_helpers.rb | 33 ++++ spec/lib/gitlab/import_export/all_models.yml | 1 + .../project_services/datadog_service_spec.rb | 179 ++++++++++++++++++ spec/models/project_spec.rb | 20 ++ spec/requests/api/services_spec.rb | 4 +- .../services_shared_context.rb | 2 + 19 files changed, 427 insertions(+), 13 deletions(-) create mode 100644 app/models/project_services/datadog_service.rb create mode 100644 changelogs/unreleased/defect-resolve-all-scroll-height.yml create mode 100644 config/feature_flags/development/datadog_ci_integration.yml create mode 100644 config/initializers/structure_load_in_transaction.rb create mode 100644 spec/models/project_services/datadog_service_spec.rb diff --git a/app/assets/javascripts/lib/utils/common_utils.js b/app/assets/javascripts/lib/utils/common_utils.js index ef25fd83db9..f88a0433535 100644 --- a/app/assets/javascripts/lib/utils/common_utils.js +++ b/app/assets/javascripts/lib/utils/common_utils.js @@ -218,26 +218,36 @@ export const isMetaKey = e => e.metaKey || e.ctrlKey || e.altKey || e.shiftKey; export const isMetaClick = e => e.metaKey || e.ctrlKey || e.which === 2; export const contentTop = () => { + const isDesktop = breakpointInstance.isDesktop(); const heightCalculators = [ () => $('#js-peek').outerHeight(), () => $('.navbar-gitlab').outerHeight(), + ({ desktop }) => { + const container = document.querySelector('.line-resolve-all-container'); + let size = 0; + + if (!desktop && container) { + size = container.offsetHeight; + } + + return size; + }, () => $('.merge-request-tabs').outerHeight(), () => $('.js-diff-files-changed').outerHeight(), - () => { - const isDesktop = breakpointInstance.isDesktop(); + ({ desktop }) => { const diffsTabIsActive = window.mrTabs?.currentAction === 'diffs'; let size; - if (isDesktop && diffsTabIsActive) { + if (desktop && diffsTabIsActive) { size = $('.diff-file .file-title-flex-parent:visible').outerHeight(); } return size; }, - () => { + ({ desktop }) => { let size; - if (breakpointInstance.isDesktop()) { + if (desktop) { size = $('.mr-version-controls').outerHeight(); } @@ -246,7 +256,7 @@ export const contentTop = () => { ]; return heightCalculators.reduce((totalHeight, calculator) => { - return totalHeight + (calculator() || 0); + return totalHeight + (calculator({ desktop: isDesktop }) || 0); }, 0); }; diff --git a/app/controllers/concerns/service_params.rb b/app/controllers/concerns/service_params.rb index a19c43a227a..c295290a123 100644 --- a/app/controllers/concerns/service_params.rb +++ b/app/controllers/concerns/service_params.rb @@ -23,6 +23,9 @@ module ServiceParams :comment_detail, :confidential_issues_events, :confluence_url, + :datadog_site, + :datadog_env, + :datadog_service, :default_irc_uri, :device, :disable_diffs, diff --git a/app/models/project.rb b/app/models/project.rb index 234126cde89..122395a6d13 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -147,6 +147,7 @@ class Project < ApplicationRecord # Project services has_one :alerts_service has_one :campfire_service + has_one :datadog_service has_one :discord_service has_one :drone_ci_service has_one :emails_on_push_service @@ -1353,6 +1354,8 @@ class Project < ApplicationRecord end def disabled_services + return ['datadog'] unless Feature.enabled?(:datadog_ci_integration, self) + [] end diff --git a/app/models/project_services/datadog_service.rb b/app/models/project_services/datadog_service.rb new file mode 100644 index 00000000000..543843ab1b0 --- /dev/null +++ b/app/models/project_services/datadog_service.rb @@ -0,0 +1,124 @@ +# frozen_string_literal: true + +class DatadogService < Service + DEFAULT_SITE = 'datadoghq.com'.freeze + URL_TEMPLATE = 'https://webhooks-http-intake.logs.%{datadog_site}/v1/input/'.freeze + URL_TEMPLATE_API_KEYS = 'https://app.%{datadog_site}/account/settings#api'.freeze + URL_API_KEYS_DOCS = "https://docs.#{DEFAULT_SITE}/account_management/api-app-keys/".freeze + + SUPPORTED_EVENTS = %w[ + pipeline job + ].freeze + + prop_accessor :datadog_site, :api_url, :api_key, :datadog_service, :datadog_env + + with_options presence: true, if: :activated? do + validates :api_key, format: { with: /\A\w+\z/ } + validates :datadog_site, format: { with: /\A[\w\.]+\z/ }, unless: :api_url + validates :api_url, public_url: true, unless: :datadog_site + end + + after_save :compose_service_hook, if: :activated? + + def self.supported_events + SUPPORTED_EVENTS + end + + def self.default_test_event + 'pipeline' + end + + def configurable_events + [] # do not allow to opt out of required hooks + end + + def title + 'Datadog' + end + + def description + 'Trace your GitLab pipelines with Datadog' + end + + def help + nil + # Maybe adding something in the future + # We could link to static help pages as well + # [More information](#{Gitlab::Routing.url_helpers.help_page_url('integration/datadog')})" + end + + def self.to_param + 'datadog' + end + + def fields + [ + { + type: 'text', name: 'datadog_site', + placeholder: DEFAULT_SITE, default: DEFAULT_SITE, + help: 'Choose the Datadog site to send data to. Set to "datadoghq.eu" to send data to the EU site', + required: false + }, + { + type: 'text', name: 'api_url', title: 'Custom URL', + help: '(Advanced) Define the full URL for your Datadog site directly', + required: false + }, + { + type: 'password', name: 'api_key', title: 'API key', + help: "API key used for authentication with Datadog", + required: true + }, + { + type: 'text', name: 'datadog_service', title: 'Service', placeholder: 'gitlab-ci', + help: 'Name of this GitLab instance that all data will be tagged with' + }, + { + type: 'text', name: 'datadog_env', title: 'Env', + help: 'The environment tag that traces will be tagged with' + } + ] + end + + def compose_service_hook + hook = service_hook || build_service_hook + hook.url = hook_url + hook.save + end + + def hook_url + url = api_url.presence || sprintf(URL_TEMPLATE, datadog_site: datadog_site) + url = URI.parse(url) + url.path = File.join(url.path || '/', api_key) + query = { service: datadog_service, env: datadog_env }.compact + url.query = query.to_query unless query.empty? + url.to_s + end + + def api_keys_url + return URL_API_KEYS_DOCS unless datadog_site.presence + + sprintf(URL_TEMPLATE_API_KEYS, datadog_site: datadog_site) + end + + def execute(data) + return if project.disabled_services.include?(to_param) + + object_kind = data[:object_kind] + object_kind = 'job' if object_kind == 'build' + return unless supported_events.include?(object_kind) + + service_hook.execute(data, "#{object_kind} hook") + end + + def test(data) + begin + result = execute(data) + return { success: false, result: result[:message] } if result[:http_status] != 200 + rescue StandardError => error + return { success: false, result: error } + end + + { success: true, result: result[:message] } + end +end diff --git a/app/models/project_services/pipelines_email_service.rb b/app/models/project_services/pipelines_email_service.rb index c11b2f7cc65..8af4cd952c9 100644 --- a/app/models/project_services/pipelines_email_service.rb +++ b/app/models/project_services/pipelines_email_service.rb @@ -40,6 +40,10 @@ class PipelinesEmailService < Service %w[pipeline] end + def self.default_test_event + 'pipeline' + end + def execute(data, force: false) return unless supported_events.include?(data[:object_kind]) return unless force || should_pipeline_be_notified?(data) diff --git a/app/models/service.rb b/app/models/service.rb index f6b98387b98..a8f98ce8b20 100644 --- a/app/models/service.rb +++ b/app/models/service.rb @@ -11,7 +11,7 @@ class Service < ApplicationRecord include EachBatch SERVICE_NAMES = %w[ - alerts asana assembla bamboo bugzilla buildkite campfire confluence custom_issue_tracker discord + alerts asana assembla bamboo bugzilla buildkite campfire confluence custom_issue_tracker datadog discord drone_ci emails_on_push ewm external_wiki flowdock hangouts_chat hipchat irker jira mattermost mattermost_slash_commands microsoft_teams packagist pipelines_email pivotaltracker prometheus pushover redmine slack slack_slash_commands teamcity unify_circuit webex_teams youtrack @@ -151,6 +151,10 @@ class Service < ApplicationRecord %w[commit push tag_push issue confidential_issue merge_request wiki_page] end + def self.default_test_event + 'push' + end + def self.event_description(event) ServicesHelper.service_event_description(event) end @@ -390,6 +394,10 @@ class Service < ApplicationRecord self.class.supported_events end + def default_test_event + self.class.default_test_event + end + def execute(data) # implement inside child end diff --git a/app/services/integrations/test/project_service.rb b/app/services/integrations/test/project_service.rb index 39471d373f9..d72ca928c34 100644 --- a/app/services/integrations/test/project_service.rb +++ b/app/services/integrations/test/project_service.rb @@ -16,9 +16,7 @@ module Integrations def data strong_memoize(:data) do - next pipeline_events_data if integration.is_a?(::PipelinesEmailService) - - case event + case event || integration.default_test_event when 'push', 'tag_push' push_events_data when 'note', 'confidential_note' @@ -37,8 +35,6 @@ module Integrations deployment_events_data when 'release' releases_events_data - else - push_events_data end end end diff --git a/changelogs/unreleased/defect-resolve-all-scroll-height.yml b/changelogs/unreleased/defect-resolve-all-scroll-height.yml new file mode 100644 index 00000000000..96b44d13e82 --- /dev/null +++ b/changelogs/unreleased/defect-resolve-all-scroll-height.yml @@ -0,0 +1,5 @@ +--- +title: Fix overscroll for MR diffs in mobile view +merge_request: 48091 +author: +type: fixed diff --git a/config/feature_flags/development/datadog_ci_integration.yml b/config/feature_flags/development/datadog_ci_integration.yml new file mode 100644 index 00000000000..c53ef36f3c1 --- /dev/null +++ b/config/feature_flags/development/datadog_ci_integration.yml @@ -0,0 +1,7 @@ +--- +name: datadog_ci_integration +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/46564 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/284088 +type: development +group: group::ecosystem +default_enabled: false diff --git a/config/initializers/structure_load_in_transaction.rb b/config/initializers/structure_load_in_transaction.rb new file mode 100644 index 00000000000..7b8f0e07203 --- /dev/null +++ b/config/initializers/structure_load_in_transaction.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +ActiveRecord::Tasks::DatabaseTasks.structure_load_flags ||= [] + +flag = '--single-transaction' + +unless ActiveRecord::Tasks::DatabaseTasks.structure_load_flags.include?(flag) + ActiveRecord::Tasks::DatabaseTasks.structure_load_flags << flag +end diff --git a/doc/api/graphql/reference/gitlab_schema.graphql b/doc/api/graphql/reference/gitlab_schema.graphql index 658dc45a722..7f445a8a8e9 100644 --- a/doc/api/graphql/reference/gitlab_schema.graphql +++ b/doc/api/graphql/reference/gitlab_schema.graphql @@ -20593,6 +20593,7 @@ enum ServiceType { CAMPFIRE_SERVICE CONFLUENCE_SERVICE CUSTOM_ISSUE_TRACKER_SERVICE + DATADOG_SERVICE DISCORD_SERVICE DRONE_CI_SERVICE EMAILS_ON_PUSH_SERVICE diff --git a/doc/api/graphql/reference/gitlab_schema.json b/doc/api/graphql/reference/gitlab_schema.json index 428c5a74b68..71e57045a2c 100644 --- a/doc/api/graphql/reference/gitlab_schema.json +++ b/doc/api/graphql/reference/gitlab_schema.json @@ -59807,6 +59807,12 @@ "isDeprecated": false, "deprecationReason": null }, + { + "name": "DATADOG_SERVICE", + "description": null, + "isDeprecated": false, + "deprecationReason": null + }, { "name": "DISCORD_SERVICE", "description": null, diff --git a/doc/api/graphql/reference/index.md b/doc/api/graphql/reference/index.md index 605b0740410..1c764e10866 100644 --- a/doc/api/graphql/reference/index.md +++ b/doc/api/graphql/reference/index.md @@ -4427,6 +4427,7 @@ State of a Sentry error. | `CAMPFIRE_SERVICE` | | | `CONFLUENCE_SERVICE` | | | `CUSTOM_ISSUE_TRACKER_SERVICE` | | +| `DATADOG_SERVICE` | | | `DISCORD_SERVICE` | | | `DRONE_CI_SERVICE` | | | `EMAILS_ON_PUSH_SERVICE` | | diff --git a/lib/api/helpers/services_helpers.rb b/lib/api/helpers/services_helpers.rb index 7bac27d1235..e77c22a1b22 100644 --- a/lib/api/helpers/services_helpers.rb +++ b/lib/api/helpers/services_helpers.rb @@ -304,6 +304,38 @@ module API desc: 'Project URL' } ], + 'datadog' => [ + { + required: true, + name: :api_key, + type: String, + desc: 'API key used for authentication with Datadog' + }, + { + required: false, + name: :datadog_site, + type: String, + desc: 'Choose the Datadog site to send data to. Set to "datadoghq.eu" to send data to the EU site' + }, + { + required: false, + name: :api_url, + type: String, + desc: '(Advanced) Define the full URL for your Datadog site directly' + }, + { + required: false, + name: :datadog_service, + type: String, + desc: 'Name of this GitLab instance that all data will be tagged with' + }, + { + required: false, + name: :datadog_env, + type: String, + desc: 'The environment tag that traces will be tagged with' + } + ], 'discord' => [ { required: true, @@ -784,6 +816,7 @@ module API ::ConfluenceService, ::CampfireService, ::CustomIssueTrackerService, + ::DatadogService, ::DiscordService, ::DroneCiService, ::EmailsOnPushService, diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index c60f916add9..412f45739b1 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -350,6 +350,7 @@ project: - services - campfire_service - confluence_service +- datadog_service - discord_service - drone_ci_service - emails_on_push_service diff --git a/spec/models/project_services/datadog_service_spec.rb b/spec/models/project_services/datadog_service_spec.rb new file mode 100644 index 00000000000..1d9f49e4824 --- /dev/null +++ b/spec/models/project_services/datadog_service_spec.rb @@ -0,0 +1,179 @@ +# frozen_string_literal: true +require 'securerandom' + +require 'spec_helper' + +RSpec.describe DatadogService, :model do + let_it_be(:project) { create(:project) } + let_it_be(:pipeline) { create(:ci_pipeline, project: project) } + let_it_be(:build) { create(:ci_build, project: project) } + + let(:active) { true } + let(:dd_site) { 'datadoghq.com' } + let(:default_url) { 'https://webhooks-http-intake.logs.datadoghq.com/v1/input/' } + let(:api_url) { nil } + let(:api_key) { SecureRandom.hex(32) } + let(:dd_env) { 'ci' } + let(:dd_service) { 'awesome-gitlab' } + + let(:expected_hook_url) { default_url + api_key + "?env=#{dd_env}&service=#{dd_service}" } + + let(:instance) do + described_class.new( + active: active, + project: project, + properties: { + datadog_site: dd_site, + api_url: api_url, + api_key: api_key, + datadog_env: dd_env, + datadog_service: dd_service + } + ) + end + + let(:saved_instance) do + instance.save! + instance + end + + let(:pipeline_data) { Gitlab::DataBuilder::Pipeline.build(pipeline) } + let(:build_data) { Gitlab::DataBuilder::Build.build(build) } + + describe 'associations' do + it { is_expected.to belong_to(:project) } + it { is_expected.to have_one(:service_hook) } + end + + describe 'validations' do + subject { instance } + + context 'when service is active' do + let(:active) { true } + + it { is_expected.to validate_presence_of(:api_key) } + it { is_expected.to allow_value(api_key).for(:api_key) } + it { is_expected.not_to allow_value('87dab2403c9d462 87aec4d9214edb1e').for(:api_key) } + it { is_expected.not_to allow_value('................................').for(:api_key) } + + context 'when selecting site' do + let(:dd_site) { 'datadoghq.com' } + let(:api_url) { nil } + + it { is_expected.to validate_presence_of(:datadog_site) } + it { is_expected.not_to validate_presence_of(:api_url) } + it { is_expected.not_to allow_value('datadog hq.com').for(:datadog_site) } + end + + context 'with custom api_url' do + let(:dd_site) { nil } + let(:api_url) { 'https://webhooks-http-intake.logs.datad0g.com/v1/input/' } + + it { is_expected.not_to validate_presence_of(:datadog_site) } + it { is_expected.to validate_presence_of(:api_url) } + it { is_expected.to allow_value(api_url).for(:api_url) } + it { is_expected.not_to allow_value('example.com').for(:api_url) } + end + + context 'when missing site and api_url' do + let(:dd_site) { nil } + let(:api_url) { nil } + + it { is_expected.not_to be_valid } + it { is_expected.to validate_presence_of(:datadog_site) } + it { is_expected.to validate_presence_of(:api_url) } + end + end + + context 'when service is not active' do + let(:active) { false } + + it { is_expected.to be_valid } + it { is_expected.not_to validate_presence_of(:api_key) } + end + end + + describe '#hook_url' do + subject { instance.hook_url } + + context 'with standard site URL' do + it { is_expected.to eq(expected_hook_url) } + end + + context 'with custom URL' do + let(:api_url) { 'https://webhooks-http-intake.logs.datad0g.com/v1/input/' } + + it { is_expected.to eq(api_url + api_key + "?env=#{dd_env}&service=#{dd_service}") } + + context 'blank' do + let(:api_url) { '' } + + it { is_expected.to eq(expected_hook_url) } + end + end + + context 'without optional params' do + let(:dd_service) { nil } + let(:dd_env) { nil } + + it { is_expected.to eq(default_url + api_key) } + end + end + + describe '#api_keys_url' do + subject { instance.api_keys_url } + + it { is_expected.to eq("https://app.#{dd_site}/account/settings#api") } + + context 'with unset datadog_site' do + let(:dd_site) { nil } + + it { is_expected.to eq("https://docs.datadoghq.com/account_management/api-app-keys/") } + end + end + + describe '#test' do + context 'when request is succesful' do + subject { saved_instance.test(pipeline_data) } + + before do + stub_request(:post, expected_hook_url).to_return(body: 'OK') + end + it { is_expected.to eq({ success: true, result: 'OK' }) } + end + + context 'when request fails' do + subject { saved_instance.test(pipeline_data) } + + before do + stub_request(:post, expected_hook_url).to_return(body: 'CRASH!!!', status: 500) + end + it { is_expected.to eq({ success: false, result: 'CRASH!!!' }) } + end + end + + describe '#execute' do + before do + stub_request(:post, expected_hook_url) + saved_instance.execute(data) + end + + context 'with pipeline data' do + let(:data) { pipeline_data } + let(:expected_headers) do + { WebHookService::GITLAB_EVENT_HEADER => 'Pipeline Hook' } + end + + it { expect(a_request(:post, expected_hook_url).with(headers: expected_headers)).to have_been_made } + end + + context 'with job data' do + let(:data) { build_data } + let(:expected_headers) do + { WebHookService::GITLAB_EVENT_HEADER => 'Job Hook' } + end + + it { expect(a_request(:post, expected_hook_url).with(headers: expected_headers)).to have_been_made } + end + end +end diff --git a/spec/models/project_spec.rb b/spec/models/project_spec.rb index fb97431599a..f9c06b20d9c 100644 --- a/spec/models/project_spec.rb +++ b/spec/models/project_spec.rb @@ -5580,6 +5580,26 @@ RSpec.describe Project, factory_default: :keep do end end + describe '#disabled_services' do + subject { build(:project).disabled_services } + + context 'without datadog_ci_integration' do + before do + stub_feature_flags(datadog_ci_integration: false) + end + + it { is_expected.to include('datadog') } + end + + context 'with datadog_ci_integration' do + before do + stub_feature_flags(datadog_ci_integration: true) + end + + it { is_expected.not_to include('datadog') } + end + end + describe '#find_or_initialize_service' do it 'avoids N+1 database queries' do allow(Service).to receive(:available_services_names).and_return(%w[prometheus pushover]) diff --git a/spec/requests/api/services_spec.rb b/spec/requests/api/services_spec.rb index 63ed57c5045..2157e69e7bf 100644 --- a/spec/requests/api/services_spec.rb +++ b/spec/requests/api/services_spec.rb @@ -76,7 +76,9 @@ RSpec.describe API::Services do required_attributes = service_attrs_list.select do |attr| service_klass.validators_on(attr).any? do |v| - v.class == ActiveRecord::Validations::PresenceValidator + v.class == ActiveRecord::Validations::PresenceValidator && + # exclude presence validators with conditional since those are not really required + ![:if, :unless].any? { |cond| v.options.include?(cond) } end end diff --git a/spec/support/shared_contexts/services_shared_context.rb b/spec/support/shared_contexts/services_shared_context.rb index 99a66bf20f6..320f7564cf9 100644 --- a/spec/support/shared_contexts/services_shared_context.rb +++ b/spec/support/shared_contexts/services_shared_context.rb @@ -16,6 +16,8 @@ Service.available_services_names.each do |service| hash.merge!(k => 'secrettoken') elsif service == 'confluence' && k == :confluence_url hash.merge!(k => 'https://example.atlassian.net/wiki') + elsif service == 'datadog' && k == :datadog_site + hash.merge!(k => 'datadoghq.com') elsif k =~ /^(.*_url|url|webhook)/ hash.merge!(k => "http://example.com") elsif service_klass.method_defined?("#{k}?")