diff options
author | Robert Speicher <rspeicher@gmail.com> | 2019-01-18 19:24:41 +0000 |
---|---|---|
committer | Robert Speicher <rspeicher@gmail.com> | 2019-01-18 19:24:41 +0000 |
commit | d7db6236d01f799abc031601b9d449a00997384a (patch) | |
tree | 87171aacd5cdfe46e87b9547b0418d851270e416 | |
parent | bbf5e92300e9c86cd1d1866fa29266f6c2bc1114 (diff) | |
parent | 6d2c02a0d79a089508922425c6aab66ef8d2d131 (diff) | |
download | gitlab-ce-d7db6236d01f799abc031601b9d449a00997384a.tar.gz |
Merge branch '11-7-stable-prepare-rc7' into '11-7-stable'
Prepare 11.7.0-rc7 release
See merge request gitlab-org/gitlab-ce!24442
68 files changed, 1673 insertions, 143 deletions
diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index 0eed1a29efd..f8f4f03b3dc 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -1.12.0 +1.12.1 diff --git a/app/assets/javascripts/error_tracking/components/error_tracking_list.vue b/app/assets/javascripts/error_tracking/components/error_tracking_list.vue new file mode 100644 index 00000000000..6981afe1ead --- /dev/null +++ b/app/assets/javascripts/error_tracking/components/error_tracking_list.vue @@ -0,0 +1,118 @@ +<script> +import { mapActions, mapState } from 'vuex'; +import { GlEmptyState, GlButton, GlLink, GlLoadingIcon, GlTable } from '@gitlab/ui'; +import Icon from '~/vue_shared/components/icon.vue'; +import TimeAgo from '~/vue_shared/components/time_ago_tooltip.vue'; +import { __ } from '~/locale'; + +export default { + fields: [ + { key: 'error', label: __('Open errors') }, + { key: 'events', label: __('Events') }, + { key: 'users', label: __('Users') }, + { key: 'lastSeen', label: __('Last seen') }, + ], + components: { + GlEmptyState, + GlButton, + GlLink, + GlLoadingIcon, + GlTable, + Icon, + TimeAgo, + }, + props: { + indexPath: { + type: String, + required: true, + }, + enableErrorTrackingLink: { + type: String, + required: true, + }, + errorTrackingEnabled: { + type: Boolean, + required: true, + }, + illustrationPath: { + type: String, + required: true, + }, + }, + computed: { + ...mapState(['errors', 'externalUrl', 'loading']), + }, + created() { + if (this.errorTrackingEnabled) { + this.startPolling(this.indexPath); + } + }, + methods: { + ...mapActions(['startPolling']), + }, +}; +</script> + +<template> + <div> + <div v-if="errorTrackingEnabled"> + <div v-if="loading" class="py-3"><gl-loading-icon :size="3" /></div> + <div v-else> + <div class="d-flex justify-content-end"> + <gl-button class="my-3 ml-auto" variant="primary" :href="externalUrl" target="_blank" + >View in Sentry <icon name="external-link" /> + </gl-button> + </div> + <gl-table + :items="errors" + :fields="$options.fields" + :show-empty="true" + :empty-text="__('No errors to display')" + > + <template slot="HEAD_events" slot-scope="data"> + <div class="text-right">{{ data.label }}</div> + </template> + <template slot="HEAD_users" slot-scope="data"> + <div class="text-right">{{ data.label }}</div> + </template> + <template slot="error" slot-scope="errors"> + <div class="d-flex flex-column"> + <div class="d-flex"> + <gl-link :href="errors.item.externalUrl" class="d-flex text-dark" target="_blank"> + <strong>{{ errors.item.title.trim() }}</strong> + <icon name="external-link" class="ml-1" /> + </gl-link> + <span class="text-secondary ml-2">{{ errors.item.culprit }}</span> + </div> + {{ errors.item.message || __('No details available') }} + </div> + </template> + + <template slot="events" slot-scope="errors"> + <div class="text-right">{{ errors.item.count }}</div> + </template> + + <template slot="users" slot-scope="errors"> + <div class="text-right">{{ errors.item.userCount }}</div> + </template> + + <template slot="lastSeen" slot-scope="errors"> + <div class="d-flex align-items-center"> + <icon name="calendar" css-classes="text-secondary mr-1" /> + <time-ago :time="errors.item.lastSeen" class="text-secondary" /> + </div> + </template> + </gl-table> + </div> + </div> + <div v-else> + <gl-empty-state + :title="__('Get started with error tracking')" + :description="__('Monitor your errors by integrating with Sentry')" + :primary-button-text="__('Enable error tracking')" + :primary-button-link="enableErrorTrackingLink" + :svg-path="illustrationPath" + /> + </div> + </div> +</template> diff --git a/app/assets/javascripts/error_tracking/index.js b/app/assets/javascripts/error_tracking/index.js new file mode 100644 index 00000000000..808ae2c9a41 --- /dev/null +++ b/app/assets/javascripts/error_tracking/index.js @@ -0,0 +1,35 @@ +import Vue from 'vue'; +import { parseBoolean } from '~/lib/utils/common_utils'; +import store from './store'; +import ErrorTrackingList from './components/error_tracking_list.vue'; + +export default () => { + if (!gon.features.errorTracking) { + return; + } + + // eslint-disable-next-line no-new + new Vue({ + el: '#js-error_tracking', + components: { + ErrorTrackingList, + }, + store, + render(createElement) { + const domEl = document.querySelector(this.$options.el); + const { indexPath, enableErrorTrackingLink, illustrationPath } = domEl.dataset; + let { errorTrackingEnabled } = domEl.dataset; + + errorTrackingEnabled = parseBoolean(errorTrackingEnabled); + + return createElement('error-tracking-list', { + props: { + indexPath, + enableErrorTrackingLink, + errorTrackingEnabled, + illustrationPath, + }, + }); + }, + }); +}; diff --git a/app/assets/javascripts/error_tracking/services/index.js b/app/assets/javascripts/error_tracking/services/index.js new file mode 100644 index 00000000000..ab89521dc46 --- /dev/null +++ b/app/assets/javascripts/error_tracking/services/index.js @@ -0,0 +1,7 @@ +import axios from '~/lib/utils/axios_utils'; + +export default { + getErrorList({ endpoint }) { + return axios.get(endpoint); + }, +}; diff --git a/app/assets/javascripts/error_tracking/store/actions.js b/app/assets/javascripts/error_tracking/store/actions.js new file mode 100644 index 00000000000..2e192c958ba --- /dev/null +++ b/app/assets/javascripts/error_tracking/store/actions.js @@ -0,0 +1,31 @@ +import Service from '../services'; +import * as types from './mutation_types'; +import createFlash from '~/flash'; +import Poll from '~/lib/utils/poll'; +import { __ } from '~/locale'; + +let eTagPoll; + +export function startPolling({ commit }, endpoint) { + eTagPoll = new Poll({ + resource: Service, + method: 'getErrorList', + data: { endpoint }, + successCallback: ({ data }) => { + if (!data) { + return; + } + commit(types.SET_ERRORS, data.errors); + commit(types.SET_EXTERNAL_URL, data.external_url); + commit(types.SET_LOADING, false); + }, + errorCallback: () => { + commit(types.SET_LOADING, false); + createFlash(__('Failed to load errors from Sentry')); + }, + }); + + eTagPoll.makeRequest(); +} + +export default () => {}; diff --git a/app/assets/javascripts/error_tracking/store/index.js b/app/assets/javascripts/error_tracking/store/index.js new file mode 100644 index 00000000000..3136682fb64 --- /dev/null +++ b/app/assets/javascripts/error_tracking/store/index.js @@ -0,0 +1,19 @@ +import Vue from 'vue'; +import Vuex from 'vuex'; +import * as actions from './actions'; +import mutations from './mutations'; + +Vue.use(Vuex); + +export const createStore = () => + new Vuex.Store({ + state: { + errors: [], + externalUrl: '', + loading: true, + }, + actions, + mutations, + }); + +export default createStore(); diff --git a/app/assets/javascripts/error_tracking/store/mutation_types.js b/app/assets/javascripts/error_tracking/store/mutation_types.js new file mode 100644 index 00000000000..f9d77a6b08e --- /dev/null +++ b/app/assets/javascripts/error_tracking/store/mutation_types.js @@ -0,0 +1,3 @@ +export const SET_ERRORS = 'SET_ERRORS'; +export const SET_EXTERNAL_URL = 'SET_EXTERNAL_URL'; +export const SET_LOADING = 'SET_LOADING'; diff --git a/app/assets/javascripts/error_tracking/store/mutations.js b/app/assets/javascripts/error_tracking/store/mutations.js new file mode 100644 index 00000000000..e4bd81db9c9 --- /dev/null +++ b/app/assets/javascripts/error_tracking/store/mutations.js @@ -0,0 +1,14 @@ +import * as types from './mutation_types'; +import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils'; + +export default { + [types.SET_ERRORS](state, data) { + state.errors = convertObjectPropsToCamelCase(data, { deep: true }); + }, + [types.SET_EXTERNAL_URL](state, url) { + state.externalUrl = url; + }, + [types.SET_LOADING](state, loading) { + state.loading = loading; + }, +}; diff --git a/app/assets/javascripts/pages/projects/error_tracking/index.js b/app/assets/javascripts/pages/projects/error_tracking/index.js new file mode 100644 index 00000000000..5a8fe137e9a --- /dev/null +++ b/app/assets/javascripts/pages/projects/error_tracking/index.js @@ -0,0 +1,5 @@ +import ErrorTracking from '~/error_tracking'; + +document.addEventListener('DOMContentLoaded', () => { + ErrorTracking(); +}); diff --git a/app/controllers/projects/error_tracking_controller.rb b/app/controllers/projects/error_tracking_controller.rb new file mode 100644 index 00000000000..4596b6c91f2 --- /dev/null +++ b/app/controllers/projects/error_tracking_controller.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true + +class Projects::ErrorTrackingController < Projects::ApplicationController + before_action :check_feature_flag! + before_action :authorize_read_sentry_issue! + before_action :push_feature_flag_to_frontend + + POLLING_INTERVAL = 10_000 + + def index + respond_to do |format| + format.html + format.json do + set_polling_interval + render_index_json + end + end + end + + private + + def render_index_json + service = ErrorTracking::ListIssuesService.new(project, current_user) + result = service.execute + + unless result[:status] == :success + return render json: { message: result[:message] }, + status: result[:http_status] || :bad_request + end + + render json: { + errors: serialize_errors(result[:issues]), + external_url: service.external_url + } + end + + def set_polling_interval + Gitlab::PollingInterval.set_header(response, interval: POLLING_INTERVAL) + end + + def serialize_errors(errors) + ErrorTracking::ErrorSerializer + .new(project: project, user: current_user) + .represent(errors) + end + + def check_feature_flag! + render_404 unless Feature.enabled?(:error_tracking, project) + end + + def push_feature_flag_to_frontend + push_frontend_feature_flag(:error_tracking, current_user) + end +end diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index 21688e54481..e3e60665506 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -178,8 +178,6 @@ class Projects::IssuesController < Projects::ApplicationController end def import_csv - return render_404 unless Feature.enabled?(:issues_import_csv) - if uploader = UploadService.new(project, params[:file]).execute ImportIssuesCsvWorker.perform_async(current_user.id, project.id, uploader.upload.id) diff --git a/app/helpers/projects/error_tracking_helper.rb b/app/helpers/projects/error_tracking_helper.rb new file mode 100644 index 00000000000..6daf2e21ca2 --- /dev/null +++ b/app/helpers/projects/error_tracking_helper.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module Projects::ErrorTrackingHelper + def error_tracking_data(project) + error_tracking_enabled = !!project.error_tracking_setting&.enabled? + + { + 'index-path' => project_error_tracking_index_path(project, + format: :json), + 'enable-error-tracking-link' => project_settings_operations_path(project), + 'error-tracking-enabled' => error_tracking_enabled.to_s, + 'illustration-path' => image_path('illustrations/cluster_popover.svg') + } + end +end diff --git a/app/helpers/projects_helper.rb b/app/helpers/projects_helper.rb index e67c327f7f8..ebbed08f78a 100644 --- a/app/helpers/projects_helper.rb +++ b/app/helpers/projects_helper.rb @@ -335,6 +335,7 @@ module ProjectsHelper builds: :read_build, clusters: :read_cluster, serverless: :read_cluster, + error_tracking: :read_sentry_issue, labels: :read_label, issues: :read_issue, project_members: :read_project_member, @@ -579,6 +580,7 @@ module ProjectsHelper environments clusters functions + error_tracking user gcp ] diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index dc6f8ae1a7f..cfdb3c0d719 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -224,8 +224,15 @@ module Ci before_transition any => [:failed] do |build| next unless build.project + next unless build.deployment - build.deployment&.drop + begin + build.deployment.drop! + rescue => e + Gitlab::Sentry.track_exception(e, extra: { build_id: build.id }) + end + + true end after_transition any => [:failed] do |build| diff --git a/app/models/error_tracking/project_error_tracking_setting.rb b/app/models/error_tracking/project_error_tracking_setting.rb index 632c64c2f1c..7f4947ba27a 100644 --- a/app/models/error_tracking/project_error_tracking_setting.rb +++ b/app/models/error_tracking/project_error_tracking_setting.rb @@ -2,13 +2,58 @@ module ErrorTracking class ProjectErrorTrackingSetting < ActiveRecord::Base + include ReactiveCaching + + self.reactive_cache_key = ->(setting) { [setting.class.model_name.singular, setting.project_id] } + belongs_to :project validates :api_url, length: { maximum: 255 }, public_url: true, url: { enforce_sanitization: true } + validate :validate_api_url_path + attr_encrypted :token, mode: :per_attribute_iv, key: Settings.attr_encrypted_db_key_base_truncated, algorithm: 'aes-256-gcm' + + after_save :clear_reactive_cache! + + def sentry_client + Sentry::Client.new(api_url, token) + end + + def sentry_external_url + self.class.extract_sentry_external_url(api_url) + end + + def list_sentry_issues(opts = {}) + with_reactive_cache('list_issues', opts.stringify_keys) do |result| + { issues: result } + end + end + + def calculate_reactive_cache(request, opts) + case request + when 'list_issues' + sentry_client.list_issues(**opts.symbolize_keys) + end + end + + # http://HOST/api/0/projects/ORG/PROJECT + # -> + # http://HOST/ORG/PROJECT + def self.extract_sentry_external_url(url) + url.sub('api/0/projects/', '') + end + + private + + def validate_api_url_path + unless URI(api_url).path.starts_with?('/api/0/projects') + errors.add(:api_url, 'path needs to start with /api/0/projects') + end + rescue URI::InvalidURIError + end end end diff --git a/app/models/remote_mirror.rb b/app/models/remote_mirror.rb index a3fa67c72bf..5eba7ddd75c 100644 --- a/app/models/remote_mirror.rb +++ b/app/models/remote_mirror.rb @@ -61,7 +61,10 @@ class RemoteMirror < ActiveRecord::Base timestamp = Time.now remote_mirror.update!( - last_update_at: timestamp, last_successful_update_at: timestamp, last_error: nil + last_update_at: timestamp, + last_successful_update_at: timestamp, + last_error: nil, + error_notification_sent: false ) end @@ -179,6 +182,10 @@ class RemoteMirror < ActiveRecord::Base project.repository.add_remote(remote_name, remote_url) end + def after_sent_notification + update_column(:error_notification_sent, true) + end + private def store_credentials @@ -221,7 +228,8 @@ class RemoteMirror < ActiveRecord::Base last_error: nil, last_update_at: nil, last_successful_update_at: nil, - update_status: 'finished' + update_status: 'finished', + error_notification_sent: false ) end diff --git a/app/models/ssh_host_key.rb b/app/models/ssh_host_key.rb index b6844dbe870..99a0c54a26a 100644 --- a/app/models/ssh_host_key.rb +++ b/app/models/ssh_host_key.rb @@ -52,6 +52,11 @@ class SshHostKey @compare_host_keys = compare_host_keys end + # Needed for reactive caching + def self.primary_key + 'id' + end + def id [project.id, url].join(':') end diff --git a/app/policies/project_policy.rb b/app/policies/project_policy.rb index d70417e710e..12f9f29dcc1 100644 --- a/app/policies/project_policy.rb +++ b/app/policies/project_policy.rb @@ -200,6 +200,7 @@ class ProjectPolicy < BasePolicy enable :read_environment enable :read_deployment enable :read_merge_request + enable :read_sentry_issue end # We define `:public_user_access` separately because there are cases in gitlab-ee diff --git a/app/serializers/error_tracking/error_entity.rb b/app/serializers/error_tracking/error_entity.rb new file mode 100644 index 00000000000..91388e7c3ad --- /dev/null +++ b/app/serializers/error_tracking/error_entity.rb @@ -0,0 +1,10 @@ +# frozen_string_literal: true + +module ErrorTracking + class ErrorEntity < Grape::Entity + expose :id, :title, :type, :user_count, :count, + :first_seen, :last_seen, :message, :culprit, + :external_url, :project_id, :project_name, :project_slug, + :short_id, :status, :frequency + end +end diff --git a/app/serializers/error_tracking/error_serializer.rb b/app/serializers/error_tracking/error_serializer.rb new file mode 100644 index 00000000000..ff9a645eb16 --- /dev/null +++ b/app/serializers/error_tracking/error_serializer.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +module ErrorTracking + class ErrorSerializer < BaseSerializer + entity ErrorEntity + end +end diff --git a/app/services/error_tracking/list_issues_service.rb b/app/services/error_tracking/list_issues_service.rb new file mode 100644 index 00000000000..4cc35cfa4a8 --- /dev/null +++ b/app/services/error_tracking/list_issues_service.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +module ErrorTracking + class ListIssuesService < ::BaseService + DEFAULT_ISSUE_STATUS = 'unresolved' + DEFAULT_LIMIT = 20 + + def execute + return error('not enabled') unless enabled? + return error('access denied') unless can_read? + + result = project_error_tracking_setting + .list_sentry_issues(issue_status: issue_status, limit: limit) + + # our results are not yet ready + unless result + return error('not ready', :no_content) + end + + success(issues: result[:issues]) + end + + def external_url + project_error_tracking_setting&.sentry_external_url + end + + private + + def project_error_tracking_setting + project.error_tracking_setting + end + + def issue_status + params[:issue_status] || DEFAULT_ISSUE_STATUS + end + + def limit + params[:limit] || DEFAULT_LIMIT + end + + def enabled? + project_error_tracking_setting&.enabled? + end + + def can_read? + can?(current_user, :read_sentry_issue, project) + end + end +end diff --git a/app/views/layouts/nav/sidebar/_project.html.haml b/app/views/layouts/nav/sidebar/_project.html.haml index c8fdc0112b4..d62cbc1684b 100644 --- a/app/views/layouts/nav/sidebar/_project.html.haml +++ b/app/views/layouts/nav/sidebar/_project.html.haml @@ -227,6 +227,12 @@ %span = _('Environments') + - if project_nav_tab?(:error_tracking) && Feature.enabled?(:error_tracking, @project) + = nav_link(controller: :error_tracking) do + = link_to project_error_tracking_index_path(@project), title: _('Error Tracking'), class: 'shortcuts-tracking qa-operations-tracking-link' do + %span + = _('Error Tracking') + - if project_nav_tab? :serverless = nav_link(controller: :functions) do = link_to project_serverless_functions_path(@project), title: _('Serverless') do diff --git a/app/views/projects/error_tracking/index.html.haml b/app/views/projects/error_tracking/index.html.haml new file mode 100644 index 00000000000..bc02c5f0e5a --- /dev/null +++ b/app/views/projects/error_tracking/index.html.haml @@ -0,0 +1,3 @@ +- page_title _('Errors') + +#js-error_tracking{ data: error_tracking_data(@project) } diff --git a/app/views/projects/issues/_nav_btns.html.haml b/app/views/projects/issues/_nav_btns.html.haml index fd6559e37ba..329efa0cdbf 100644 --- a/app/views/projects/issues/_nav_btns.html.haml +++ b/app/views/projects/issues/_nav_btns.html.haml @@ -1,5 +1,5 @@ - show_feed_buttons = local_assigns.fetch(:show_feed_buttons, true) -- show_import_button = local_assigns.fetch(:show_import_button, true) && Feature.enabled?(:issues_import_csv) && can?(current_user, :import_issues, @project) +- show_import_button = local_assigns.fetch(:show_import_button, true) && can?(current_user, :import_issues, @project) - show_export_button = local_assigns.fetch(:show_export_button, true) .nav-controls.issues-nav-controls diff --git a/app/views/shared/_personal_access_tokens_created_container.html.haml b/app/views/shared/_personal_access_tokens_created_container.html.haml index 3150d39b84a..a8d3de66418 100644 --- a/app/views/shared/_personal_access_tokens_created_container.html.haml +++ b/app/views/shared/_personal_access_tokens_created_container.html.haml @@ -6,7 +6,7 @@ = container_title .form-group .input-group - = text_field_tag 'created-personal-access-token', new_token_value, readonly: true, class: "form-control js-select-on-focus", 'aria-describedby' => "created-token-help-block" + = text_field_tag 'created-personal-access-token', new_token_value, readonly: true, class: "qa-created-personal-access-token form-control js-select-on-focus", 'aria-describedby' => "created-token-help-block" %span.input-group-append = clipboard_button(text: new_token_value, title: clipboard_button_title, placement: "left", class: "input-group-text btn-default btn-clipboard") %span#created-token-help-block.form-text.text-muted.text-danger Make sure you save it - you won't be able to access it again. diff --git a/app/views/shared/_personal_access_tokens_form.html.haml b/app/views/shared/_personal_access_tokens_form.html.haml index f4df7bdcd83..0891b3459ec 100644 --- a/app/views/shared/_personal_access_tokens_form.html.haml +++ b/app/views/shared/_personal_access_tokens_form.html.haml @@ -12,7 +12,7 @@ .row .form-group.col-md-6 = f.label :name, class: 'label-bold' - = f.text_field :name, class: "form-control", required: true + = f.text_field :name, class: "form-control qa-personal-access-token-name-field", required: true .row .form-group.col-md-6 @@ -26,4 +26,4 @@ = render 'shared/tokens/scopes_form', prefix: 'personal_access_token', token: token, scopes: scopes .prepend-top-default - = f.submit "Create #{type} token", class: "btn btn-success" + = f.submit "Create #{type} token", class: "btn btn-success qa-create-token-button" diff --git a/app/views/shared/_personal_access_tokens_table.html.haml b/app/views/shared/_personal_access_tokens_table.html.haml index 2efd03d4867..49f3aae0f98 100644 --- a/app/views/shared/_personal_access_tokens_table.html.haml +++ b/app/views/shared/_personal_access_tokens_table.html.haml @@ -29,7 +29,7 @@ %span.token-never-expires-label Never %td= token.scopes.present? ? token.scopes.join(", ") : "<no scopes selected>" - path = impersonation ? revoke_admin_user_impersonation_token_path(token.user, token) : revoke_profile_personal_access_token_path(token) - %td= link_to "Revoke", path, method: :put, class: "btn btn-danger float-right", data: { confirm: "Are you sure you want to revoke this #{type} Token? This action cannot be undone." } + %td= link_to "Revoke", path, method: :put, class: "btn btn-danger float-right qa-revoke-button", data: { confirm: "Are you sure you want to revoke this #{type} Token? This action cannot be undone." } - else .settings-message.text-center This user has no active #{type} Tokens. diff --git a/app/views/shared/empty_states/_issues.html.haml b/app/views/shared/empty_states/_issues.html.haml index 0434860dec4..9c0fa6b8994 100644 --- a/app/views/shared/empty_states/_issues.html.haml +++ b/app/views/shared/empty_states/_issues.html.haml @@ -1,6 +1,6 @@ - button_path = local_assigns.fetch(:button_path, false) - project_select_button = local_assigns.fetch(:project_select_button, false) -- show_import_button = local_assigns.fetch(:show_import_button, false) && Feature.enabled?(:issues_import_csv) && can?(current_user, :import_issues, @project) +- show_import_button = local_assigns.fetch(:show_import_button, false) && can?(current_user, :import_issues, @project) - has_button = button_path || project_select_button .row.empty-state diff --git a/app/views/shared/tokens/_scopes_form.html.haml b/app/views/shared/tokens/_scopes_form.html.haml index af9db5f59a8..a5d3e1c8de0 100644 --- a/app/views/shared/tokens/_scopes_form.html.haml +++ b/app/views/shared/tokens/_scopes_form.html.haml @@ -4,6 +4,6 @@ - scopes.each do |scope| %fieldset.form-group.form-check - = check_box_tag "#{prefix}[scopes][]", scope, token.scopes.include?(scope), id: "#{prefix}_scopes_#{scope}", class: 'form-check-input' + = check_box_tag "#{prefix}[scopes][]", scope, token.scopes.include?(scope), id: "#{prefix}_scopes_#{scope}", class: "form-check-input qa-#{scope}-radio" = label_tag ("#{prefix}_scopes_#{scope}"), scope, class: 'label-bold form-check-label' .text-secondary= t scope, scope: [:doorkeeper, :scope_desc] diff --git a/app/workers/reactive_caching_worker.rb b/app/workers/reactive_caching_worker.rb index 96ff8cd6222..7c66ac046ea 100644 --- a/app/workers/reactive_caching_worker.rb +++ b/app/workers/reactive_caching_worker.rb @@ -12,7 +12,7 @@ class ReactiveCachingWorker end return unless klass - klass.find_by(id: id).try(:exclusively_update_reactive_cache!, *args) + klass.find_by(klass.primary_key => id).try(:exclusively_update_reactive_cache!, *args) end # rubocop: enable CodeReuse/ActiveRecord end diff --git a/app/workers/remote_mirror_notification_worker.rb b/app/workers/remote_mirror_notification_worker.rb index 70c2e857d09..5bafe8e2046 100644 --- a/app/workers/remote_mirror_notification_worker.rb +++ b/app/workers/remote_mirror_notification_worker.rb @@ -9,7 +9,10 @@ class RemoteMirrorNotificationWorker # We check again if there's an error because a newer run since this job was # fired could've completed successfully. return unless remote_mirror && remote_mirror.last_error.present? + return if remote_mirror.error_notification_sent? NotificationService.new.remote_mirror_update_failed(remote_mirror) + + remote_mirror.after_sent_notification end end diff --git a/changelogs/unreleased/error_tracking_feature_flag_fe.yml b/changelogs/unreleased/error_tracking_feature_flag_fe.yml new file mode 100644 index 00000000000..607929eb6b8 --- /dev/null +++ b/changelogs/unreleased/error_tracking_feature_flag_fe.yml @@ -0,0 +1,5 @@ +--- +title: Display a list of Sentry Issues in GitLab +merge_request: 23770 +author: +type: added diff --git a/changelogs/unreleased/fix-runner-eternal-loop-when-update-job-result.yml b/changelogs/unreleased/fix-runner-eternal-loop-when-update-job-result.yml new file mode 100644 index 00000000000..5a6c36e6f5f --- /dev/null +++ b/changelogs/unreleased/fix-runner-eternal-loop-when-update-job-result.yml @@ -0,0 +1,5 @@ +--- +title: Fix runner eternal loop when update job result +merge_request: 24481 +author: +type: fixed diff --git a/changelogs/unreleased/pl-reactive-caching-primary_key.yml b/changelogs/unreleased/pl-reactive-caching-primary_key.yml new file mode 100644 index 00000000000..a72933c19b1 --- /dev/null +++ b/changelogs/unreleased/pl-reactive-caching-primary_key.yml @@ -0,0 +1,5 @@ +--- +title: Enable caching for records which primary key is not `id` +merge_request: 24245 +author: +type: fixed diff --git a/changelogs/unreleased/upgrade-gitaly-1-12-1.yml b/changelogs/unreleased/upgrade-gitaly-1-12-1.yml new file mode 100644 index 00000000000..4759a2dd8d8 --- /dev/null +++ b/changelogs/unreleased/upgrade-gitaly-1-12-1.yml @@ -0,0 +1,5 @@ +--- +title: Upgrade to gitaly 1.12.1 +merge_request: 24361 +author: +type: fixed diff --git a/config/routes/project.rb b/config/routes/project.rb index cf5a57300cf..e6ecb4bc9d8 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -442,6 +442,8 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do end end + resources :error_tracking, only: [:index], controller: :error_tracking + # Since both wiki and repository routing contains wildcard characters # its preferable to keep it below all other project routes draw :wiki diff --git a/db/migrate/20190115054216_add_error_notification_sent_to_remote_mirrors.rb b/db/migrate/20190115054216_add_error_notification_sent_to_remote_mirrors.rb new file mode 100644 index 00000000000..d8f979a1848 --- /dev/null +++ b/db/migrate/20190115054216_add_error_notification_sent_to_remote_mirrors.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +class AddErrorNotificationSentToRemoteMirrors < ActiveRecord::Migration[5.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + def change + add_column :remote_mirrors, :error_notification_sent, :boolean + end +end diff --git a/db/schema.rb b/db/schema.rb index 87826881d58..c4902116a3a 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20190103140724) do +ActiveRecord::Schema.define(version: 20190115054216) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -1849,6 +1849,7 @@ ActiveRecord::Schema.define(version: 20190103140724) do t.string "encrypted_credentials_salt" t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.boolean "error_notification_sent" t.index ["last_successful_update_at"], name: "index_remote_mirrors_on_last_successful_update_at", using: :btree t.index ["project_id"], name: "index_remote_mirrors_on_project_id", using: :btree end diff --git a/doc/api/oauth2.md b/doc/api/oauth2.md index 8a1e6b52092..dfe62554852 100644 --- a/doc/api/oauth2.md +++ b/doc/api/oauth2.md @@ -1,126 +1,165 @@ # GitLab as an OAuth2 provider -This document covers using the [OAuth2](https://oauth.net/2/) protocol to allow other services access GitLab resources on user's behalf. +This document covers using the [OAuth2](https://oauth.net/2/) protocol to allow +other services to access GitLab resources on user's behalf. -If you want GitLab to be an OAuth authentication service provider to sign into other services please see the [OAuth2 provider](../integration/oauth_provider.md) -documentation. +If you want GitLab to be an OAuth authentication service provider to sign into +other services, see the [OAuth2 provider](../integration/oauth_provider.md) +documentation. This functionality is based on the +[doorkeeper Ruby gem](https://github.com/doorkeeper-gem/doorkeeper). -This functionality is based on [doorkeeper gem](https://github.com/doorkeeper-gem/doorkeeper). +## Supported OAuth2 flows -## Supported OAuth2 Flows +GitLab currently supports the following authorization flows: -GitLab currently supports following authorization flows: +- **Web application flow:** Most secure and common type of flow, designed for + applications with secure server-side. +- **Implicit grant flow:** This flow is designed for user-agent only apps (e.g., single + page web application running on GitLab Pages). +- **Resource owner password credentials flow:** To be used **only** for securely + hosted, first-party services. -* *Web Application Flow* - Most secure and common type of flow, designed for the applications with secure server-side. -* *Implicit Flow* - This flow is designed for user-agent only apps (e.g. single page web application running on GitLab Pages). -* *Resource Owner Password Credentials Flow* - To be used **only** for securely hosted, first-party services. +Refer to the [OAuth RFC](https://tools.ietf.org/html/rfc6749) to find out +how all those flows work and pick the right one for your use case. -Please refer to [OAuth RFC](https://tools.ietf.org/html/rfc6749) to find out in details how all those flows work and pick the right one for your use case. +Both **web application** and **implicit grant** flows require `application` to be +registered first via the `/profile/applications` page in your user's account. +During registration, by enabling proper scopes, you can limit the range of +resources which the `application` can access. Upon creation, you'll obtain the +`application` credentials: _Application ID_ and _Client Secret_ - **keep them secure**. -Both *web application* and *implicit* flows require `application` to be registered first via `/profile/applications` page -in your user's account. During registration, by enabling proper scopes you can limit the range of resources which the `application` can access. Upon creation -you'll obtain `application` credentials: _Application ID_ and _Client Secret_ - **keep them secure**. +CAUTION: **Important:** +OAuth specification advises sending the `state` parameter with each request to +`/oauth/authorize`. We highly recommended sending a unique value with each request +and validate it against the one in the redirect request. This is important in +order to prevent [CSRF attacks](https://www.owasp.org/index.php/Cross-Site_Request_Forgery_(CSRF)). +The `state` parameter really should have been a requirement in the standard! ->**Important:** OAuth specification advises sending `state` parameter with each request to `/oauth/authorize`. We highly recommended to send a unique -value with each request and validate it against the one in redirect request. This is important to prevent [CSRF attacks]. The `state` param really should -have been a requirement in the standard! +In the following sections you will find detailed instructions on how to obtain +authorization with each flow. -In the following sections you will find detailed instructions on how to obtain authorization with each flow. +### Web application flow -### Web Application Flow +NOTE: **Note:** +Check the [RFC spec](https://tools.ietf.org/html/rfc6749#section-4.1) for a +detailed flow description. -Check [RFC spec](http://tools.ietf.org/html/rfc6749#section-4.1) for a detailed flow description +The web application flow is: -#### 1. Requesting authorization code +1. Request authorization code. To do that, you should redirect the user to the + `/oauth/authorize` endpoint with the following GET parameters: -To request the authorization code, you should redirect the user to the `/oauth/authorize` endpoint with following GET parameters: + ``` + https://gitlab.example.com/oauth/authorize?client_id=APP_ID&redirect_uri=REDIRECT_URI&response_type=code&state=YOUR_UNIQUE_STATE_HASH + ``` -``` -https://gitlab.example.com/oauth/authorize?client_id=APP_ID&redirect_uri=REDIRECT_URI&response_type=code&state=YOUR_UNIQUE_STATE_HASH -``` + This will ask the user to approve the applications access to their account and + then redirect back to the `REDIRECT_URI` you provided. The redirect will + include the GET `code` parameter, for example: -This will ask the user to approve the applications access to their account and then redirect back to the `REDIRECT_URI` you provided. The redirect will -include the GET `code` parameter, for example: + ``` + http://myapp.com/oauth/redirect?code=1234567890&state=YOUR_UNIQUE_STATE_HASH + ``` -`http://myapp.com/oauth/redirect?code=1234567890&state=YOUR_UNIQUE_STATE_HASH` + You should then use `code` to request an access token. -You should then use the `code` to request an access token. +1. Once you have the authorization code you can request an `access_token` using the + code. You can do that by using any HTTP client. In the following example, + we are using Ruby's `rest-client`: -#### 2. Requesting access token + ```ruby + parameters = 'client_id=APP_ID&client_secret=APP_SECRET&code=RETURNED_CODE&grant_type=authorization_code&redirect_uri=REDIRECT_URI' + RestClient.post 'http://gitlab.example.com/oauth/token', parameters + ``` -Once you have the authorization code you can request an `access_token` using the code, to do that you can use any HTTP client. In the following example, -we are using Ruby's `rest-client`: + Example response: -``` -parameters = 'client_id=APP_ID&client_secret=APP_SECRET&code=RETURNED_CODE&grant_type=authorization_code&redirect_uri=REDIRECT_URI' -RestClient.post 'http://gitlab.example.com/oauth/token', parameters + ```json + { + "access_token": "de6780bc506a0446309bd9362820ba8aed28aa506c71eedbe1c5c4f9dd350e54", + "token_type": "bearer", + "expires_in": 7200, + "refresh_token": "8257e65c97202ed1726cf9571600918f3bffb2544b26e00a61df9897668c33a1" + } + ``` -# The response will be -{ - "access_token": "de6780bc506a0446309bd9362820ba8aed28aa506c71eedbe1c5c4f9dd350e54", - "token_type": "bearer", - "expires_in": 7200, - "refresh_token": "8257e65c97202ed1726cf9571600918f3bffb2544b26e00a61df9897668c33a1" -} -``` ->**Note:** -The `redirect_uri` must match the `redirect_uri` used in the original authorization request. +NOTE: **Note:** +The `redirect_uri` must match the `redirect_uri` used in the original +authorization request. You can now make requests to the API with the access token returned. +### Implicit grant flow -### Implicit Grant - -Check [RFC spec](http://tools.ietf.org/html/rfc6749#section-4.2) for a detailed flow description. - -Unlike the web flow, the client receives an `access token` immediately as a result of the authorization request. The flow does not use client secret -or authorization code because all of the application code and storage is easily accessible, therefore __secrets__ can leak easily. +NOTE: **Note:** +Check the [RFC spec](https://tools.ietf.org/html/rfc6749#section-4.2) for a +detailed flow description. ->**Important:** Avoid using this flow for applications that store data outside of the GitLab instance. If you do, make sure to verify `application id` -associated with access token before granting access to the data -(see [/oauth/token/info](https://github.com/doorkeeper-gem/doorkeeper/wiki/API-endpoint-descriptions-and-examples#get----oauthtokeninfo)). - +CAUTION: **Important:** +Avoid using this flow for applications that store data outside of the GitLab +instance. If you do, make sure to verify `application id` associated with the +access token before granting access to the data +(see [/oauth/token/info](https://github.com/doorkeeper-gem/doorkeeper/wiki/API-endpoint-descriptions-and-examples#get----oauthtokeninfo)). -#### 1. Requesting access token +Unlike the web flow, the client receives an `access token` immediately as a +result of the authorization request. The flow does not use the client secret +or the authorization code because all of the application code and storage is +easily accessible, therefore secrets can leak easily. -To request the access token, you should redirect the user to the `/oauth/authorize` endpoint using `token` response type: +To request the access token, you should redirect the user to the +`/oauth/authorize` endpoint using `token` response type: ``` https://gitlab.example.com/oauth/authorize?client_id=APP_ID&redirect_uri=REDIRECT_URI&response_type=token&state=YOUR_UNIQUE_STATE_HASH ``` -This will ask the user to approve the applications access to their account and then redirect back to the `REDIRECT_URI` you provided. The redirect -will include a fragment with `access_token` as well as token details in GET parameters, for example: +This will ask the user to approve the application's access to their account and +then redirect them back to the `REDIRECT_URI` you provided. The redirect +will include a fragment with `access_token` as well as token details in GET +parameters, for example: ``` http://myapp.com/oauth/redirect#access_token=ABCDExyz123&state=YOUR_UNIQUE_STATE_HASH&token_type=bearer&expires_in=3600 ``` -### Resource Owner Password Credentials +### Resource owner password credentials flow -Check [RFC spec](http://tools.ietf.org/html/rfc6749#section-4.3) for a detailed flow description. +NOTE: **Note:** +Check the [RFC spec](https://tools.ietf.org/html/rfc6749#section-4.3) for a +detailed flow description. -> **Deprecation notice:** Starting in GitLab 8.11, the Resource Owner Password Credentials has been *disabled* for users with two-factor authentication -turned on. These users can access the API using [personal access tokens] instead. +NOTE: **Note:** +The Resource Owner Password Credentials is disabled for users with [two-factor +authentication](../user/profile/account/two_factor_authentication.md) turned on. +These users can access the API using [personal access tokens](../user/profile/personal_access_tokens.md) +instead. -In this flow, a token is requested in exchange for the resource owner credentials (username and password). -The credentials should only be used when there is a high degree of trust between the resource owner and the client (e.g. the -client is part of the device operating system or a highly privileged application), and when other authorization grant types are not -available (such as an authorization code). +In this flow, a token is requested in exchange for the resource owner credentials +(username and password). ->**Important:** -Never store the users credentials and only use this grant type when your client is deployed to a trusted environment, in 99% of cases [personal access tokens] -are a better choice. +The credentials should only be used when: -Even though this grant type requires direct client access to the resource owner credentials, the resource owner credentials are used -for a single request and are exchanged for an access token. This grant type can eliminate the need for the client to store the -resource owner credentials for future use, by exchanging the credentials with a long-lived access token or refresh token. +- There is a high degree of trust between the resource owner and the client. For + example, the client is part of the device operating system or a highly + privileged application. +- Other authorization grant types are not available (such as an authorization code). -#### 1. Requesting access token +CAUTION: **Important:** +Never store the user's credentials and only use this grant type when your client +is deployed to a trusted environment, in 99% of cases +[personal access tokens](../user/profile/personal_access_tokens.md) are a better +choice. -POST request to `/oauth/token` with parameters: +Even though this grant type requires direct client access to the resource owner +credentials, the resource owner credentials are used for a single request and +are exchanged for an access token. This grant type can eliminate the need for +the client to store the resource owner credentials for future use, by exchanging +the credentials with a long-lived access token or refresh token. -``` +To request an access token, you must make a POST request to `/oauth/token` with +the following parameters: + +```json { "grant_type" : "password", "username" : "user@example.com", @@ -128,6 +167,13 @@ POST request to `/oauth/token` with parameters: } ``` +Example cURL request: + +```sh +echo 'grant_type=password&username=<your_username>&password=<your_password>' > auth.txt +curl --data "@auth.txt" --request POST https://gitlab.example.com/oauth/token +``` + Then, you'll receive the access token back in the response: ``` @@ -138,7 +184,7 @@ Then, you'll receive the access token back in the response: } ``` -For testing you can use the oauth2 ruby gem: +For testing, you can use the `oauth2` Ruby gem: ``` client = OAuth2::Client.new('the_client_id', 'the_client_secret', :site => "http://example.com") @@ -148,7 +194,9 @@ puts access_token.token ## Access GitLab API with `access token` -The `access token` allows you to make requests to the API on a behalf of a user. You can pass the token either as GET parameter +The `access token` allows you to make requests to the API on behalf of a user. +You can pass the token either as GET parameter: + ``` GET https://gitlab.example.com/api/v4/user?access_token=OAUTH-TOKEN ``` @@ -158,6 +206,3 @@ or you can put the token to the Authorization header: ``` curl --header "Authorization: Bearer OAUTH-TOKEN" https://gitlab.example.com/api/v4/user ``` - -[personal access tokens]: ../user/profile/personal_access_tokens.md -[CSRF attacks]: http://www.oauthsecurity.com/#user-content-authorization-code-flow
\ No newline at end of file diff --git a/lib/gitlab/error_tracking/error.rb b/lib/gitlab/error_tracking/error.rb new file mode 100644 index 00000000000..4af5192aa6a --- /dev/null +++ b/lib/gitlab/error_tracking/error.rb @@ -0,0 +1,14 @@ +# frozen_string_literal: true + +module Gitlab + module ErrorTracking + class Error + include ActiveModel::Model + + attr_accessor :id, :title, :type, :user_count, :count, + :first_seen, :last_seen, :message, :culprit, + :external_url, :project_id, :project_name, :project_slug, + :short_id, :status, :frequency + end + end +end diff --git a/lib/sentry/client.rb b/lib/sentry/client.rb new file mode 100644 index 00000000000..343f2c49a7f --- /dev/null +++ b/lib/sentry/client.rb @@ -0,0 +1,104 @@ +# frozen_string_literal: true + +module Sentry + class Client + Error = Class.new(StandardError) + + attr_accessor :url, :token + + def initialize(api_url, token) + @url = api_url + @token = token + end + + def list_issues(issue_status:, limit:) + issues = get_issues(issue_status: issue_status, limit: limit) + map_to_errors(issues) + end + + private + + def request_params + { + headers: { + 'Authorization' => "Bearer #{@token}" + }, + follow_redirects: false + } + end + + def get_issues(issue_status:, limit:) + resp = Gitlab::HTTP.get( + issues_api_url, + **request_params.merge(query: { + query: "is:#{issue_status}", + limit: limit + }) + ) + + handle_response(resp) + end + + def handle_response(response) + unless response.code == 200 + raise Client::Error, "Sentry response error: #{response.code}" + end + + response.as_json + end + + def issues_api_url + issues_url = URI(@url + '/issues/') + issues_url.path.squeeze!('/') + + issues_url + end + + def map_to_errors(issues) + issues.map do |issue| + map_to_error(issue) + end + end + + def issue_url(id) + issues_url = @url + "/issues/#{id}" + issues_url = ErrorTracking::ProjectErrorTrackingSetting.extract_sentry_external_url(issues_url) + + uri = URI(issues_url) + uri.path.squeeze!('/') + + uri.to_s + end + + def map_to_error(issue) + id = issue.fetch('id') + project = issue.fetch('project') + + count = issue.fetch('count', nil) + + frequency = issue.dig('stats', '24h') + message = issue.dig('metadata', 'value') + + external_url = issue_url(id) + + Gitlab::ErrorTracking::Error.new( + id: id, + first_seen: issue.fetch('firstSeen', nil), + last_seen: issue.fetch('lastSeen', nil), + title: issue.fetch('title', nil), + type: issue.fetch('type', nil), + user_count: issue.fetch('userCount', nil), + count: count, + message: message, + culprit: issue.fetch('culprit', nil), + external_url: external_url, + short_id: issue.fetch('shortId', nil), + status: issue.fetch('status', nil), + frequency: frequency, + project_id: project.fetch('id'), + project_name: project.fetch('name', nil), + project_slug: project.fetch('slug', nil) + ) + end + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 1782c9d5e4d..7daaf5f94b2 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -2746,6 +2746,9 @@ msgstr "" msgid "Enable and configure Prometheus metrics." msgstr "" +msgid "Enable error tracking" +msgstr "" + msgid "Enable for this project" msgstr "" @@ -2953,6 +2956,9 @@ msgstr "" msgid "Error while loading the merge request. Please try again." msgstr "" +msgid "Errors" +msgstr "" + msgid "Estimated" msgstr "" @@ -2974,6 +2980,9 @@ msgstr "" msgid "EventFilterBy|Filter by team" msgstr "" +msgid "Events" +msgstr "" + msgid "Every %{action} attempt has failed: %{job_error_message}. Please try again." msgstr "" @@ -3061,6 +3070,9 @@ msgstr "" msgid "Failed to load emoji list." msgstr "" +msgid "Failed to load errors from Sentry" +msgstr "" + msgid "Failed to remove issue from board, please try again." msgstr "" @@ -3244,6 +3256,9 @@ msgstr "" msgid "Geo" msgstr "" +msgid "Get started with error tracking" +msgstr "" + msgid "Getting started with releases" msgstr "" @@ -3950,6 +3965,9 @@ msgstr "" msgid "Last reply by" msgstr "" +msgid "Last seen" +msgstr "" + msgid "Last update" msgstr "" @@ -4339,6 +4357,9 @@ msgstr "" msgid "Modal|Close" msgstr "" +msgid "Monitor your errors by integrating with Sentry" +msgstr "" + msgid "Monitoring" msgstr "" @@ -4503,9 +4524,15 @@ msgstr "" msgid "No contributions were found" msgstr "" +msgid "No details available" +msgstr "" + msgid "No due date" msgstr "" +msgid "No errors to display" +msgstr "" + msgid "No estimate or time spent" msgstr "" @@ -4724,6 +4751,9 @@ msgstr "" msgid "Open comment type dropdown" msgstr "" +msgid "Open errors" +msgstr "" + msgid "Open in Xcode" msgstr "" diff --git a/qa/Rakefile b/qa/Rakefile new file mode 100644 index 00000000000..8df1cfdc174 --- /dev/null +++ b/qa/Rakefile @@ -0,0 +1,6 @@ +require_relative 'qa/tools/revoke_all_personal_access_tokens' + +desc "Revokes all personal access tokens" +task :revoke_personal_access_tokens do + QA::Tools::RevokeAllPersonalAccessTokens.new.run +end diff --git a/qa/qa/page/profile/personal_access_tokens.rb b/qa/qa/page/profile/personal_access_tokens.rb index 9191dbe9cf3..8c12eff5cf1 100644 --- a/qa/qa/page/profile/personal_access_tokens.rb +++ b/qa/qa/page/profile/personal_access_tokens.rb @@ -3,29 +3,51 @@ module QA module Profile class PersonalAccessTokens < Page::Base view 'app/views/shared/_personal_access_tokens_form.html.haml' do - element :personal_access_token_name_field, 'text_field :name' # rubocop:disable QA/ElementWithPattern - element :create_token_button, 'submit "Create #{type} token"' # rubocop:disable QA/ElementWithPattern, Lint/InterpolationCheck - element :scopes_api_radios, "label :scopes" # rubocop:disable QA/ElementWithPattern + element :personal_access_token_name_field + element :create_token_button + end + + view 'app/views/shared/tokens/_scopes_form.html.haml' do + element :api_radio, 'qa-#{scope}-radio' # rubocop:disable QA/ElementWithPattern, Lint/InterpolationCheck end view 'app/views/shared/_personal_access_tokens_created_container.html.haml' do - element :create_token_field, "text_field_tag 'created-personal-access-token'" # rubocop:disable QA/ElementWithPattern + element :created_personal_access_token + end + view 'app/views/shared/_personal_access_tokens_table.html.haml' do + element :revoke_button end def fill_token_name(name) - fill_in 'personal_access_token_name', with: name + fill_element(:personal_access_token_name_field, name) end def check_api - check 'personal_access_token_scopes_api' + check_element(:api_radio) end def create_token - click_on 'Create personal access token' + click_element(:create_token_button) end def created_access_token - page.find('#created-personal-access-token').value + find_element(:created_personal_access_token, wait: 30).value + end + + def has_token_row_for_name?(token_name) + page.has_css?('tr', text: token_name, wait: 1.0) + end + + def first_token_row_for_name(token_name) + page.find('tr', text: token_name, match: :first, wait: 1.0) + end + + def revoke_first_token_with_name(token_name) + within first_token_row_for_name(token_name) do + accept_confirm do + click_element(:revoke_button) + end + end end end end diff --git a/qa/qa/resource/base.rb b/qa/qa/resource/base.rb index dcea144ab74..f325162d1c0 100644 --- a/qa/qa/resource/base.rb +++ b/qa/qa/resource/base.rb @@ -126,10 +126,6 @@ module QA mod end - def self.attributes_names - dynamic_attributes.instance_methods(false).sort.grep_v(/=$/) - end - class DSL def initialize(base) @base = base diff --git a/qa/qa/resource/user.rb b/qa/qa/resource/user.rb index c26f0c84a1f..b9580d81171 100644 --- a/qa/qa/resource/user.rb +++ b/qa/qa/resource/user.rb @@ -6,9 +6,12 @@ module QA module Resource class User < Base attr_reader :unique_id - attr_writer :username, :password, :name, :email + attr_writer :username, :password attr_accessor :provider, :extern_uid + attribute :name + attribute :email + def initialize @unique_id = SecureRandom.hex(8) end @@ -22,11 +25,11 @@ module QA end def name - @name ||= username + @name ||= api_resource&.dig(:name) || username end def email - @email ||= "#{username}@example.com" + @email ||= api_resource&.dig(:email) || "#{username}@example.com" end def credentials_given? diff --git a/qa/qa/specs/features/browser_ui/3_create/repository/user_views_raw_diff_patch_requests_spec.rb b/qa/qa/specs/features/browser_ui/3_create/repository/user_views_raw_diff_patch_requests_spec.rb index 203338ddf77..3a5d89e6b83 100644 --- a/qa/qa/specs/features/browser_ui/3_create/repository/user_views_raw_diff_patch_requests_spec.rb +++ b/qa/qa/specs/features/browser_ui/3_create/repository/user_views_raw_diff_patch_requests_spec.rb @@ -39,11 +39,15 @@ module QA end it 'user views raw email patch' do + user = Resource::User.fabricate_via_api! do |user| + user.username = Runtime::User.username + end + view_commit Page::Project::Commit::Show.perform(&:select_email_patches) - expect(page).to have_content('From: Administrator <admin@example.com>') + expect(page).to have_content("From: #{user.name} <#{user.email}>") expect(page).to have_content('Subject: [PATCH] Add second file') expect(page).to have_content('diff --git a/second b/second') end diff --git a/qa/qa/tools/revoke_all_personal_access_tokens.rb b/qa/qa/tools/revoke_all_personal_access_tokens.rb new file mode 100644 index 00000000000..7484b633bf6 --- /dev/null +++ b/qa/qa/tools/revoke_all_personal_access_tokens.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +require_relative '../../qa' +require 'net/protocol.rb' +# This script revokes all personal access tokens with the name of 'api-test-token' on the host specified by GITLAB_ADDRESS +# Required environment variables: GITLAB_USERNAME, GITLAB_PASSWORD and GITLAB_ADDRESS +# Run `rake revoke_personal_access_tokens` + +module QA + module Tools + class RevokeAllPersonalAccessTokens + def run + do_run + rescue Net::ReadTimeout + STDOUT.puts 'Net::ReadTimeout during run. Trying again' + run + end + + private + + def do_run + raise ArgumentError, "Please provide GITLAB_USERNAME" unless ENV['GITLAB_USERNAME'] + raise ArgumentError, "Please provide GITLAB_PASSWORD" unless ENV['GITLAB_PASSWORD'] + raise ArgumentError, "Please provide GITLAB_ADDRESS" unless ENV['GITLAB_ADDRESS'] + + STDOUT.puts 'Running...' + + Runtime::Browser.visit(ENV['GITLAB_ADDRESS'], Page::Main::Login) + Page::Main::Login.perform(&:sign_in_using_credentials) + Page::Main::Menu.perform(&:go_to_profile_settings) + Page::Profile::Menu.perform(&:click_access_tokens) + + token_name = 'api-test-token' + + Page::Profile::PersonalAccessTokens.perform do |page| + while page.has_token_row_for_name?(token_name) + page.revoke_first_token_with_name(token_name) + print "\e[32m.\e[0m" + end + end + end + end + end +end diff --git a/qa/spec/resource/base_spec.rb b/qa/spec/resource/base_spec.rb index dc9e16792d3..b8c406ae72a 100644 --- a/qa/spec/resource/base_spec.rb +++ b/qa/spec/resource/base_spec.rb @@ -138,10 +138,6 @@ describe QA::Resource::Base do describe '.attribute' do include_context 'simple resource' - it 'appends new attribute' do - expect(subject.attributes_names).to eq([:no_block, :test, :web_url]) - end - context 'when the attribute is populated via a block' do it 'returns value from the block' do result = subject.fabricate!(resource: resource) diff --git a/spec/controllers/projects/error_tracking_controller_spec.rb b/spec/controllers/projects/error_tracking_controller_spec.rb new file mode 100644 index 00000000000..729e71b87a6 --- /dev/null +++ b/spec/controllers/projects/error_tracking_controller_spec.rb @@ -0,0 +1,142 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe Projects::ErrorTrackingController do + set(:project) { create(:project) } + set(:user) { create(:user) } + + before do + sign_in(user) + project.add_maintainer(user) + end + + describe 'GET #index' do + describe 'html' do + it 'renders index with 200 status code' do + get :index, params: project_params + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to render_template(:index) + end + + context 'with feature flag disabled' do + before do + stub_feature_flags(error_tracking: false) + end + + it 'returns 404' do + get :index, params: project_params + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'with insufficient permissions' do + before do + project.add_guest(user) + end + + it 'returns 404' do + get :index, params: project_params + + expect(response).to have_gitlab_http_status(:not_found) + end + end + + context 'with an anonymous user' do + before do + sign_out(user) + end + + it 'redirects to sign-in page' do + get :index, params: project_params + + expect(response).to redirect_to(new_user_session_path) + end + end + end + + describe 'format json' do + shared_examples 'no data' do + it 'returns no data' do + get :index, params: project_params(format: :json) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('error_tracking/index') + expect(json_response['external_url']).to be_nil + expect(json_response['errors']).to eq([]) + end + end + + let(:list_issues_service) { spy(:list_issues_service) } + let(:external_url) { 'http://example.com' } + + before do + expect(ErrorTracking::ListIssuesService) + .to receive(:new).with(project, user) + .and_return(list_issues_service) + end + + context 'service result is successful' do + before do + expect(list_issues_service).to receive(:execute) + .and_return(status: :success, issues: [error]) + expect(list_issues_service).to receive(:external_url) + .and_return(external_url) + end + + let(:error) { build(:error_tracking_error) } + + it 'returns a list of errors' do + get :index, params: project_params(format: :json) + + expect(response).to have_gitlab_http_status(:ok) + expect(response).to match_response_schema('error_tracking/index') + expect(json_response['external_url']).to eq(external_url) + expect(json_response['errors']).to eq([error].as_json) + end + end + + context 'service result is erroneous' do + let(:error_message) { 'error message' } + + context 'without http_status' do + before do + expect(list_issues_service).to receive(:execute) + .and_return(status: :error, message: error_message) + end + + it 'returns 400 with message' do + get :index, params: project_params(format: :json) + + expect(response).to have_gitlab_http_status(:bad_request) + expect(json_response['message']).to eq(error_message) + end + end + + context 'with explicit http_status' do + let(:http_status) { :no_content } + + before do + expect(list_issues_service).to receive(:execute) + .and_return(status: :error, message: error_message, http_status: http_status) + end + + it 'returns http_status with message' do + get :index, params: project_params(format: :json) + + expect(response).to have_gitlab_http_status(http_status) + expect(json_response['message']).to eq(error_message) + end + end + end + end + end + + private + + def project_params(opts = {}) + opts.reverse_merge(namespace_id: project.namespace, project_id: project) + end +end diff --git a/spec/controllers/projects/issues_controller_spec.rb b/spec/controllers/projects/issues_controller_spec.rb index df21dc7bc85..35cf4ea4c68 100644 --- a/spec/controllers/projects/issues_controller_spec.rb +++ b/spec/controllers/projects/issues_controller_spec.rb @@ -1030,19 +1030,6 @@ describe Projects::IssuesController do let(:project) { create(:project, :public) } let(:file) { fixture_file_upload('spec/fixtures/csv_comma.csv') } - context 'feature disabled' do - it 'returns 404' do - sign_in(user) - project.add_maintainer(user) - - stub_feature_flags(issues_import_csv: false) - - import_csv - - expect(response).to have_gitlab_http_status :not_found - end - end - context 'unauthorized' do it 'returns 404 for guests' do sign_out(:user) diff --git a/spec/factories/error_tracking/error.rb b/spec/factories/error_tracking/error.rb new file mode 100644 index 00000000000..ff883a3d22c --- /dev/null +++ b/spec/factories/error_tracking/error.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +FactoryBot.define do + factory :error_tracking_error, class: Gitlab::ErrorTracking::Error do + id 'id' + title 'title' + type 'error' + user_count 1 + count 2 + first_seen { Time.now } + last_seen { Time.now } + message 'message' + culprit 'culprit' + external_url 'http://example.com/id' + project_id 'project1' + project_name 'project name' + project_slug 'project_name' + short_id 'ID' + status 'unresolved' + frequency [] + + skip_create + end +end diff --git a/spec/factories/project_error_tracking_settings.rb b/spec/factories/project_error_tracking_settings.rb index f044cbe8755..fbd8dfd395c 100644 --- a/spec/factories/project_error_tracking_settings.rb +++ b/spec/factories/project_error_tracking_settings.rb @@ -3,7 +3,7 @@ FactoryBot.define do factory :project_error_tracking_setting, class: ErrorTracking::ProjectErrorTrackingSetting do project - api_url 'https://gitlab.com' + api_url 'https://gitlab.com/api/0/projects/sentry-org/sentry-project' enabled true token 'access_token_123' end diff --git a/spec/fixtures/api/schemas/error_tracking/error.json b/spec/fixtures/api/schemas/error_tracking/error.json new file mode 100644 index 00000000000..df2c02d7d5d --- /dev/null +++ b/spec/fixtures/api/schemas/error_tracking/error.json @@ -0,0 +1,21 @@ +{ + "type": "object", + "required" : [ + "external_url", + "last_seen", + "message", + "type" + ], + "properties" : { + "id": { "type": "string"}, + "first_seen": { "type": "string", "format": "date-time" }, + "last_seen": { "type": "string", "format": "date-time" }, + "type": { "type": "string" }, + "message": { "type": "string" }, + "culprit": { "type": "string" }, + "count": { "type": "integer"}, + "external_url": { "type": "string" }, + "user_count": { "type": "integer"} + }, + "additionalProperties": true +} diff --git a/spec/fixtures/api/schemas/error_tracking/index.json b/spec/fixtures/api/schemas/error_tracking/index.json new file mode 100644 index 00000000000..d3abc29ffa7 --- /dev/null +++ b/spec/fixtures/api/schemas/error_tracking/index.json @@ -0,0 +1,15 @@ +{ + "type": "object", + "required": [ + "external_url", + "errors" + ], + "properties": { + "external_url": { "type": ["string", "null"] }, + "errors": { + "type": "array", + "items": { "$ref": "error.json" } + } + }, + "additionalProperties": false +} diff --git a/spec/fixtures/sentry/issues_sample_response.json b/spec/fixtures/sentry/issues_sample_response.json new file mode 100644 index 00000000000..ed22499cfa1 --- /dev/null +++ b/spec/fixtures/sentry/issues_sample_response.json @@ -0,0 +1,42 @@ +[{ + "lastSeen": "2018-12-31T12:00:11Z", + "numComments": 0, + "userCount": 0, + "stats": { + "24h": [ + [ + 1546437600, + 0 + ] + ] + }, + "culprit": "sentry.tasks.reports.deliver_organization_user_report", + "title": "gaierror: [Errno -2] Name or service not known", + "id": "11", + "assignedTo": null, + "logger": null, + "type": "error", + "annotations": [], + "metadata": { + "type": "gaierror", + "value": "[Errno -2] Name or service not known" + }, + "status": "unresolved", + "subscriptionDetails": null, + "isPublic": false, + "hasSeen": false, + "shortId": "INTERNAL-4", + "shareId": null, + "firstSeen": "2018-12-17T12:00:14Z", + "count": "21", + "permalink": "35.228.54.90/sentry/internal/issues/11/", + "level": "error", + "isSubscribed": true, + "isBookmarked": false, + "project": { + "slug": "internal", + "id": "1", + "name": "Internal" + }, + "statusDetails": {} + }] diff --git a/spec/helpers/projects/error_tracking_helper_spec.rb b/spec/helpers/projects/error_tracking_helper_spec.rb new file mode 100644 index 00000000000..7516a636c93 --- /dev/null +++ b/spec/helpers/projects/error_tracking_helper_spec.rb @@ -0,0 +1,58 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Projects::ErrorTrackingHelper do + include Gitlab::Routing.url_helpers + + set(:project) { create(:project) } + + describe '#error_tracking_data' do + let(:setting_path) { project_settings_operations_path(project) } + + let(:index_path) do + project_error_tracking_index_path(project, format: :json) + end + + context 'without error_tracking_setting' do + it 'returns frontend configuration' do + expect(error_tracking_data(project)).to eq( + 'index-path' => index_path, + 'enable-error-tracking-link' => setting_path, + 'error-tracking-enabled' => 'false', + "illustration-path" => "/images/illustrations/cluster_popover.svg" + ) + end + end + + context 'with error_tracking_setting' do + let(:error_tracking_setting) do + create(:project_error_tracking_setting, project: project) + end + + context 'when enabled' do + before do + error_tracking_setting.update!(enabled: true) + end + + it 'show error tracking enabled' do + expect(error_tracking_data(project)).to include( + 'error-tracking-enabled' => 'true' + ) + end + end + + context 'when disabled' do + before do + error_tracking_setting.update!(enabled: false) + end + + it 'show error tracking not enabled' do + expect(error_tracking_data(project)).to include( + 'error-tracking-enabled' => 'false' + ) + end + end + end + end +end diff --git a/spec/javascripts/error_tracking/components/error_tracking_list_spec.js b/spec/javascripts/error_tracking/components/error_tracking_list_spec.js new file mode 100644 index 00000000000..08bbb390993 --- /dev/null +++ b/spec/javascripts/error_tracking/components/error_tracking_list_spec.js @@ -0,0 +1,100 @@ +import { createLocalVue, shallowMount } from '@vue/test-utils'; +import Vuex from 'vuex'; +import ErrorTrackingList from '~/error_tracking/components/error_tracking_list.vue'; +import { GlButton, GlEmptyState, GlLoadingIcon, GlTable } from '@gitlab/ui'; + +const localVue = createLocalVue(); +localVue.use(Vuex); + +describe('ErrorTrackingList', () => { + let store; + let wrapper; + + function mountComponent({ errorTrackingEnabled = true } = {}) { + wrapper = shallowMount(ErrorTrackingList, { + localVue, + store, + propsData: { + indexPath: '/path', + enableErrorTrackingLink: '/link', + errorTrackingEnabled, + illustrationPath: 'illustration/path', + }, + }); + } + + beforeEach(() => { + const actions = { + getErrorList: () => {}, + }; + + const state = { + errors: [], + loading: true, + }; + + store = new Vuex.Store({ + actions, + state, + }); + }); + + afterEach(() => { + if (wrapper) { + wrapper.destroy(); + } + }); + + describe('loading', () => { + beforeEach(() => { + mountComponent(); + }); + + it('shows spinner', () => { + expect(wrapper.find(GlLoadingIcon).exists()).toBeTruthy(); + expect(wrapper.find(GlTable).exists()).toBeFalsy(); + expect(wrapper.find(GlButton).exists()).toBeFalsy(); + }); + }); + + describe('results', () => { + beforeEach(() => { + store.state.loading = false; + + mountComponent(); + }); + + it('shows table', () => { + expect(wrapper.find(GlLoadingIcon).exists()).toBeFalsy(); + expect(wrapper.find(GlTable).exists()).toBeTruthy(); + expect(wrapper.find(GlButton).exists()).toBeTruthy(); + }); + }); + + describe('no results', () => { + beforeEach(() => { + store.state.loading = false; + + mountComponent(); + }); + + it('shows empty table', () => { + expect(wrapper.find(GlLoadingIcon).exists()).toBeFalsy(); + expect(wrapper.find(GlTable).exists()).toBeTruthy(); + expect(wrapper.find(GlButton).exists()).toBeTruthy(); + }); + }); + + describe('error tracking feature disabled', () => { + beforeEach(() => { + mountComponent({ errorTrackingEnabled: false }); + }); + + it('shows empty state', () => { + expect(wrapper.find(GlEmptyState).exists()).toBeTruthy(); + expect(wrapper.find(GlLoadingIcon).exists()).toBeFalsy(); + expect(wrapper.find(GlTable).exists()).toBeFalsy(); + expect(wrapper.find(GlButton).exists()).toBeFalsy(); + }); + }); +}); diff --git a/spec/javascripts/error_tracking/store/mutation_spec.js b/spec/javascripts/error_tracking/store/mutation_spec.js new file mode 100644 index 00000000000..8117104bdbc --- /dev/null +++ b/spec/javascripts/error_tracking/store/mutation_spec.js @@ -0,0 +1,36 @@ +import mutations from '~/error_tracking/store/mutations'; +import * as types from '~/error_tracking/store/mutation_types'; + +describe('Error tracking mutations', () => { + describe('SET_ERRORS', () => { + let state; + + beforeEach(() => { + state = { errors: [] }; + }); + + it('camelizes response', () => { + const errors = [ + { + title: 'the title', + external_url: 'localhost:3456', + count: 100, + userCount: 10, + }, + ]; + + mutations[types.SET_ERRORS](state, errors); + + expect(state).toEqual({ + errors: [ + { + title: 'the title', + externalUrl: 'localhost:3456', + count: 100, + userCount: 10, + }, + ], + }); + }); + }); +}); diff --git a/spec/lib/sentry/client_spec.rb b/spec/lib/sentry/client_spec.rb new file mode 100644 index 00000000000..b36be0fd9c1 --- /dev/null +++ b/spec/lib/sentry/client_spec.rb @@ -0,0 +1,119 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe Sentry::Client do + let(:issue_status) { 'unresolved' } + let(:limit) { 20 } + let(:sentry_url) { 'https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project' } + let(:token) { 'test-token' } + + let(:sample_response) do + Gitlab::Utils.deep_indifferent_access( + JSON.parse(File.read(Rails.root.join('spec/fixtures/sentry/issues_sample_response.json'))) + ) + end + + subject(:client) { described_class.new(sentry_url, token) } + + describe '#list_issues' do + subject { client.list_issues(issue_status: issue_status, limit: limit) } + + before do + stub_sentry_request(sentry_url + '/issues/?limit=20&query=is:unresolved', body: sample_response) + end + + it 'returns objects of type ErrorTracking::Error' do + expect(subject.length).to eq(1) + expect(subject[0]).to be_a(Gitlab::ErrorTracking::Error) + end + + context 'error object created from sentry response' do + using RSpec::Parameterized::TableSyntax + + where(:error_object, :sentry_response) do + :id | :id + :first_seen | :firstSeen + :last_seen | :lastSeen + :title | :title + :type | :type + :user_count | :userCount + :count | :count + :message | [:metadata, :value] + :culprit | :culprit + :short_id | :shortId + :status | :status + :frequency | [:stats, '24h'] + :project_id | [:project, :id] + :project_name | [:project, :name] + :project_slug | [:project, :slug] + end + + with_them do + it { expect(subject[0].public_send(error_object)).to eq(sample_response[0].dig(*sentry_response)) } + end + + context 'external_url' do + it 'is constructed correctly' do + expect(subject[0].external_url).to eq('https://sentrytest.gitlab.com/sentry-org/sentry-project/issues/11') + end + end + end + + context 'redirects' do + let(:redirect_to) { 'https://redirected.example.com' } + let(:other_url) { 'https://other.example.org' } + + let!(:redirected_req_stub) { stub_sentry_request(other_url) } + + let!(:redirect_req_stub) do + stub_sentry_request( + sentry_url + '/issues/?limit=20&query=is:unresolved', + status: 302, + headers: { location: redirect_to } + ) + end + + it 'does not follow redirects' do + expect { subject }.to raise_exception(Sentry::Client::Error, 'Sentry response error: 302') + expect(redirect_req_stub).to have_been_requested + expect(redirected_req_stub).not_to have_been_requested + end + end + + # Sentry API returns 404 if there are extra slashes in the URL! + context 'extra slashes in URL' do + let(:sentry_url) { 'https://sentrytest.gitlab.com/api/0/projects//sentry-org/sentry-project/' } + let(:client) { described_class.new(sentry_url, token) } + + let!(:valid_req_stub) do + stub_sentry_request( + 'https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project/' \ + 'issues/?limit=20&query=is:unresolved' + ) + end + + it 'removes extra slashes in api url' do + expect(Gitlab::HTTP).to receive(:get).with( + URI('https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project/issues/'), + anything + ).and_call_original + + client.list_issues(issue_status: issue_status, limit: limit) + + expect(valid_req_stub).to have_been_requested + end + end + end + + private + + def stub_sentry_request(url, body: {}, status: 200, headers: {}) + WebMock.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/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index 1afc2436bb5..60d89313f07 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -3028,6 +3028,24 @@ describe Ci::Build do subject.drop! end end + + context 'when associated deployment failed to update its status' do + let(:build) { create(:ci_build, :running, pipeline: pipeline) } + let!(:deployment) { create(:deployment, deployable: build) } + + before do + allow_any_instance_of(Deployment) + .to receive(:drop!).and_raise('Unexpected error') + end + + it 'can drop the build' do + expect(Gitlab::Sentry).to receive(:track_exception) + + expect { build.drop! }.not_to raise_error + + expect(build).to be_failed + end + end end describe '.matches_tag_ids' do diff --git a/spec/models/error_tracking/project_error_tracking_setting_spec.rb b/spec/models/error_tracking/project_error_tracking_setting_spec.rb index 83f29718eda..2f8ab21d4b2 100644 --- a/spec/models/error_tracking/project_error_tracking_setting_spec.rb +++ b/spec/models/error_tracking/project_error_tracking_setting_spec.rb @@ -3,33 +3,106 @@ require 'spec_helper' describe ErrorTracking::ProjectErrorTrackingSetting do + include ReactiveCachingHelpers + set(:project) { create(:project) } + subject { create(:project_error_tracking_setting, project: project) } + describe 'Associations' do it { is_expected.to belong_to(:project) } end describe 'Validations' do - subject { create(:project_error_tracking_setting, project: project) } - context 'when api_url is over 255 chars' do - before do + it 'fails validation' do subject.api_url = 'https://' + 'a' * 250 - end - it 'fails validation' do expect(subject).not_to be_valid expect(subject.errors.messages[:api_url]).to include('is too long (maximum is 255 characters)') end end context 'With unsafe url' do - let(:project_error_tracking_setting) { create(:project_error_tracking_setting, project: project) } - it 'fails validation' do - project_error_tracking_setting.api_url = "https://replaceme.com/'><script>alert(document.cookie)</script>" + subject.api_url = "https://replaceme.com/'><script>alert(document.cookie)</script>" + + expect(subject).not_to be_valid + end + end + + context 'URL path' do + it 'fails validation with wrong path' do + subject.api_url = 'http://gitlab.com/project1/something' + + expect(subject).not_to be_valid + expect(subject.errors.messages[:api_url]).to include('path needs to start with /api/0/projects') + end + + it 'passes validation with correct path' do + subject.api_url = 'http://gitlab.com/api/0/projects/project1/something' + + expect(subject).to be_valid + end + end + end + + describe '#sentry_external_url' do + let(:sentry_url) { 'https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project' } + + before do + subject.api_url = sentry_url + end + + it 'returns the correct url' do + expect(subject.class).to receive(:extract_sentry_external_url).with(sentry_url).and_call_original + + result = subject.sentry_external_url + + expect(result).to eq('https://sentrytest.gitlab.com/sentry-org/sentry-project') + end + end + + describe '#sentry_client' do + it 'returns sentry client' do + expect(subject.sentry_client).to be_a(Sentry::Client) + end + end + + describe '#list_sentry_issues' do + let(:issues) { [:list, :of, :issues] } + + let(:opts) do + { issue_status: 'unresolved', limit: 10 } + end + + let(:result) do + subject.list_sentry_issues(**opts) + end + + context 'when cached' do + let(:sentry_client) { spy(:sentry_client) } + + before do + stub_reactive_cache(subject, issues, opts) + synchronous_reactive_cache(subject) + + expect(subject).to receive(:sentry_client).and_return(sentry_client) + end + + it 'returns cached issues' do + expect(sentry_client).to receive(:list_issues).with(opts) + .and_return(issues) + + expect(result).to eq(issues: issues) + end + end + + context 'when not cached' do + it 'returns nil' do + expect(subject).not_to receive(:sentry_client) - expect(project_error_tracking_setting).not_to be_valid + expect(result).to be_nil end end end diff --git a/spec/models/remote_mirror_spec.rb b/spec/models/remote_mirror_spec.rb index 224bc9ed935..c06e9a08ab4 100644 --- a/spec/models/remote_mirror_spec.rb +++ b/spec/models/remote_mirror_spec.rb @@ -303,6 +303,25 @@ describe RemoteMirror, :mailer do end end + context '#url=' do + let(:remote_mirror) { create(:project, :repository, :remote_mirror).remote_mirrors.first } + + it 'resets all the columns when URL changes' do + remote_mirror.update(last_error: Time.now, + last_update_at: Time.now, + last_successful_update_at: Time.now, + update_status: 'started', + error_notification_sent: true) + + expect { remote_mirror.update_attribute(:url, 'http://new.example.com') } + .to change { remote_mirror.last_error }.to(nil) + .and change { remote_mirror.last_update_at }.to(nil) + .and change { remote_mirror.last_successful_update_at }.to(nil) + .and change { remote_mirror.update_status }.to('finished') + .and change { remote_mirror.error_notification_sent }.to(false) + end + end + context '#updated_since?' do let(:remote_mirror) { create(:project, :repository, :remote_mirror).remote_mirrors.first } let(:timestamp) { Time.now - 5.minutes } diff --git a/spec/policies/project_policy_spec.rb b/spec/policies/project_policy_spec.rb index 9cb20854f6e..2a4030de998 100644 --- a/spec/policies/project_policy_spec.rb +++ b/spec/policies/project_policy_spec.rb @@ -24,7 +24,7 @@ describe ProjectPolicy do download_code fork_project create_project_snippet update_issue admin_issue admin_label admin_list read_commit_status read_build read_container_image read_pipeline read_environment read_deployment - read_merge_request download_wiki_code + read_merge_request download_wiki_code read_sentry_issue ] end diff --git a/spec/services/error_tracking/list_issues_service_spec.rb b/spec/services/error_tracking/list_issues_service_spec.rb new file mode 100644 index 00000000000..d9dab1d705c --- /dev/null +++ b/spec/services/error_tracking/list_issues_service_spec.rb @@ -0,0 +1,87 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe ErrorTracking::ListIssuesService do + set(:user) { create(:user) } + set(:project) { create(:project) } + + let(:sentry_url) { 'https://sentrytest.gitlab.com/api/0/projects/sentry-org/sentry-project' } + let(:token) { 'test-token' } + let(:result) { subject.execute } + + let(:error_tracking_setting) do + create(:project_error_tracking_setting, api_url: sentry_url, token: token, project: project) + end + + subject { described_class.new(project, user) } + + before do + expect(project).to receive(:error_tracking_setting).at_least(:once).and_return(error_tracking_setting) + + project.add_reporter(user) + end + + describe '#execute' do + context 'with authorized user' do + context 'when list_sentry_issues returns issues' do + let(:issues) { [:list, :of, :issues] } + + before do + expect(error_tracking_setting) + .to receive(:list_sentry_issues).and_return(issues: issues) + end + + it 'returns the issues' do + expect(result).to eq(status: :success, issues: issues) + end + end + + context 'when list_sentry_issues returns nil' do + before do + expect(error_tracking_setting) + .to receive(:list_sentry_issues).and_return(nil) + end + + it 'result is not ready' do + expect(result).to eq( + status: :error, http_status: :no_content, message: 'not ready') + end + end + end + + context 'with unauthorized user' do + let(:unauthorized_user) { create(:user) } + + subject { described_class.new(project, unauthorized_user) } + + it 'returns error' do + result = subject.execute + + expect(result).to include(status: :error, message: 'access denied') + end + end + + context 'with error tracking disabled' do + before do + error_tracking_setting.enabled = false + end + + it 'raises error' do + result = subject.execute + + expect(result).to include(status: :error, message: 'not enabled') + end + end + end + + describe '#sentry_external_url' do + let(:external_url) { 'https://sentrytest.gitlab.com/sentry-org/sentry-project' } + + it 'calls ErrorTracking::ProjectErrorTrackingSetting' do + expect(error_tracking_setting).to receive(:sentry_external_url).and_call_original + + subject.external_url + end + end +end diff --git a/spec/services/projects/operations/update_service_spec.rb b/spec/services/projects/operations/update_service_spec.rb index 731be907453..6afae3da80c 100644 --- a/spec/services/projects/operations/update_service_spec.rb +++ b/spec/services/projects/operations/update_service_spec.rb @@ -17,7 +17,7 @@ describe Projects::Operations::UpdateService do { error_tracking_setting_attributes: { enabled: false, - api_url: 'http://url', + api_url: 'http://gitlab.com/api/0/projects/org/project', token: 'token' } } @@ -32,7 +32,7 @@ describe Projects::Operations::UpdateService do project.reload expect(project.error_tracking_setting).not_to be_enabled - expect(project.error_tracking_setting.api_url).to eq('http://url') + expect(project.error_tracking_setting.api_url).to eq('http://gitlab.com/api/0/projects/org/project') expect(project.error_tracking_setting.token).to eq('token') end end @@ -42,7 +42,7 @@ describe Projects::Operations::UpdateService do { error_tracking_setting_attributes: { enabled: true, - api_url: 'http://url', + api_url: 'http://gitlab.com/api/0/projects/org/project', token: 'token' } } @@ -52,7 +52,7 @@ describe Projects::Operations::UpdateService do expect(result[:status]).to eq(:success) expect(project.error_tracking_setting).to be_enabled - expect(project.error_tracking_setting.api_url).to eq('http://url') + expect(project.error_tracking_setting.api_url).to eq('http://gitlab.com/api/0/projects/org/project') expect(project.error_tracking_setting.token).to eq('token') end end diff --git a/spec/workers/remote_mirror_notification_worker_spec.rb b/spec/workers/remote_mirror_notification_worker_spec.rb new file mode 100644 index 00000000000..e3db10ed645 --- /dev/null +++ b/spec/workers/remote_mirror_notification_worker_spec.rb @@ -0,0 +1,39 @@ +require 'spec_helper' + +describe RemoteMirrorNotificationWorker, :mailer do + set(:project) { create(:project, :repository, :remote_mirror) } + set(:mirror) { project.remote_mirrors.first } + + describe '#execute' do + it 'calls NotificationService#remote_mirror_update_failed when the mirror exists' do + mirror.update_column(:last_error, "There was a problem fetching") + + expect(NotificationService).to receive_message_chain(:new, :remote_mirror_update_failed) + + subject.perform(mirror.id) + + expect(mirror.reload.error_notification_sent?).to be_truthy + end + + it 'does nothing when the mirror has no errors' do + expect(NotificationService).not_to receive(:new) + + subject.perform(mirror.id) + end + + it 'does nothing when the mirror does not exist' do + expect(NotificationService).not_to receive(:new) + + subject.perform(RemoteMirror.maximum(:id).to_i.succ) + end + + it 'does nothing when a notification has already been sent' do + mirror.update_columns(last_error: "There was a problem fetching", + error_notification_sent: true) + + expect(NotificationService).not_to receive(:new) + + subject.perform(mirror.id) + end + end +end diff --git a/spec/workers/repository_update_remote_mirror_worker_spec.rb b/spec/workers/repository_update_remote_mirror_worker_spec.rb index d73b0b53713..b582a3650b6 100644 --- a/spec/workers/repository_update_remote_mirror_worker_spec.rb +++ b/spec/workers/repository_update_remote_mirror_worker_spec.rb @@ -22,6 +22,13 @@ describe RepositoryUpdateRemoteMirrorWorker do expect { subject.perform(remote_mirror.id, Time.now) }.to change { remote_mirror.reload.update_status }.to('finished') end + it 'resets the notification flag upon success' do + expect_any_instance_of(Projects::UpdateRemoteMirrorService).to receive(:execute).with(remote_mirror).and_return(status: :success) + remote_mirror.update_column(:error_notification_sent, true) + + expect { subject.perform(remote_mirror.id, Time.now) }.to change { remote_mirror.reload.error_notification_sent }.to(false) + end + it 'sets status as failed when update remote mirror service executes with errors' do error_message = 'fail!' |