diff options
| -rw-r--r-- | app/helpers/application_settings_helper.rb | 4 | ||||
| -rw-r--r-- | app/models/application_setting.rb | 13 | ||||
| -rw-r--r-- | app/models/application_setting_implementation.rb | 21 | ||||
| -rw-r--r-- | app/views/admin/application_settings/_logging.html.haml | 38 | ||||
| -rw-r--r-- | app/views/admin/application_settings/reporting.html.haml | 11 | ||||
| -rw-r--r-- | app/views/layouts/_head.html.haml | 2 | ||||
| -rw-r--r-- | changelogs/unreleased/refactor-sentry.yml | 5 | ||||
| -rw-r--r-- | config/initializers/sentry.rb | 13 | ||||
| -rw-r--r-- | db/post_migrate/remove_sentry_from_application_settings.rb | 36 | ||||
| -rw-r--r-- | doc/api/settings.md | 4 | ||||
| -rw-r--r-- | lib/api/settings.rb | 8 | ||||
| -rw-r--r-- | lib/gitlab/gon_helper.rb | 4 | ||||
| -rw-r--r-- | lib/gitlab/sentry.rb | 2 | ||||
| -rw-r--r-- | locale/gitlab.pot | 6 | ||||
| -rw-r--r-- | spec/features/raven_js_spec.rb | 2 | ||||
| -rw-r--r-- | spec/models/application_setting_spec.rb | 30 | ||||
| -rw-r--r-- | spec/requests/api/helpers_spec.rb | 6 | ||||
| -rw-r--r-- | spec/support/helpers/stub_configuration.rb | 6 | ||||
| -rw-r--r-- | spec/support/shared_examples/application_setting_examples.rb | 39 |
19 files changed, 61 insertions, 189 deletions
diff --git a/app/helpers/application_settings_helper.rb b/app/helpers/application_settings_helper.rb index d837c42fd68..aaaa954047f 100644 --- a/app/helpers/application_settings_helper.rb +++ b/app/helpers/application_settings_helper.rb @@ -165,8 +165,6 @@ module ApplicationSettingsHelper :authorized_keys_enabled, :auto_devops_enabled, :auto_devops_domain, - :clientside_sentry_dsn, - :clientside_sentry_enabled, :container_registry_token_expire_delay, :default_artifacts_expire_in, :default_branch_protection, @@ -235,8 +233,6 @@ module ApplicationSettingsHelper :restricted_visibility_levels, :rsa_key_restriction, :send_user_confirmation_email, - :sentry_dsn, - :sentry_enabled, :session_expire_delay, :shared_runners_enabled, :shared_runners_text, diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb index bbe2d2e8fd4..cd645850af3 100644 --- a/app/models/application_setting.rb +++ b/app/models/application_setting.rb @@ -30,6 +30,10 @@ class ApplicationSetting < ApplicationRecord ignore_column :circuitbreaker_check_interval ignore_column :koding_url ignore_column :koding_enabled + ignore_column :sentry_enabled + ignore_column :sentry_dsn + ignore_column :clientside_sentry_enabled + ignore_column :clientside_sentry_dsn cache_markdown_field :sign_in_text cache_markdown_field :help_page_text @@ -75,14 +79,6 @@ class ApplicationSetting < ApplicationRecord presence: true, if: :recaptcha_enabled - validates :sentry_dsn, - presence: true, - if: :sentry_enabled - - validates :clientside_sentry_dsn, - presence: true, - if: :clientside_sentry_enabled - validates :akismet_api_key, presence: true, if: :akismet_enabled @@ -264,7 +260,6 @@ class ApplicationSetting < ApplicationRecord encode: true before_validation :ensure_uuid! - before_validation :strip_sentry_values before_save :ensure_runners_registration_token before_save :ensure_health_check_access_token diff --git a/app/models/application_setting_implementation.rb b/app/models/application_setting_implementation.rb index cf328bcd994..df4caed175d 100644 --- a/app/models/application_setting_implementation.rb +++ b/app/models/application_setting_implementation.rb @@ -180,27 +180,6 @@ module ApplicationSettingImplementation super(levels&.map { |level| Gitlab::VisibilityLevel.level_value(level) }) end - def strip_sentry_values - sentry_dsn.strip! if sentry_dsn.present? - clientside_sentry_dsn.strip! if clientside_sentry_dsn.present? - end - - def sentry_enabled - Gitlab.config.sentry.enabled || read_attribute(:sentry_enabled) - end - - def sentry_dsn - Gitlab.config.sentry.dsn || read_attribute(:sentry_dsn) - end - - def clientside_sentry_enabled - Gitlab.config.sentry.enabled || read_attribute(:clientside_sentry_enabled) - end - - def clientside_sentry_dsn - Gitlab.config.sentry.clientside_dsn || read_attribute(:clientside_sentry_dsn) - end - def performance_bar_allowed_group Group.find_by_id(performance_bar_allowed_group_id) end diff --git a/app/views/admin/application_settings/_logging.html.haml b/app/views/admin/application_settings/_logging.html.haml deleted file mode 100644 index d57066bba01..00000000000 --- a/app/views/admin/application_settings/_logging.html.haml +++ /dev/null @@ -1,38 +0,0 @@ -= form_for @application_setting, url: reporting_admin_application_settings_path(anchor: 'js-logging-settings'), html: { class: 'fieldset-form' } do |f| - = form_errors(@application_setting) - - %p - %strong - NOTE: - These settings will be removed from the UI in a GitLab 12.0 release and made available within gitlab.yml. - In addition, you will be able to define a Sentry Environment to differentiate between multiple deployments. For example, development, staging, and production. - - %fieldset - .form-group - .form-check - = f.check_box :sentry_enabled, class: 'form-check-input' - = f.label :sentry_enabled, class: 'form-check-label' do - Enable Sentry - .form-text.text-muted - %p This setting requires a restart to take effect. - Sentry is an error reporting and logging tool which is currently not shipped with GitLab, get it here: - %a{ href: 'https://getsentry.com', target: '_blank', rel: 'noopener noreferrer' } https://getsentry.com - - .form-group - = f.label :sentry_dsn, 'Sentry DSN', class: 'label-bold' - = f.text_field :sentry_dsn, class: 'form-control' - - .form-group - .form-check - = f.check_box :clientside_sentry_enabled, class: 'form-check-input' - = f.label :clientside_sentry_enabled, class: 'form-check-label' do - Enable Clientside Sentry - .form-text.text-muted - Sentry can also be used for reporting and logging clientside exceptions. - %a{ href: 'https://sentry.io/for/javascript/', target: '_blank', rel: 'noopener noreferrer' } https://sentry.io/for/javascript/ - - .form-group - = f.label :clientside_sentry_dsn, 'Clientside Sentry DSN', class: 'label-bold' - = f.text_field :clientside_sentry_dsn, class: 'form-control' - - = f.submit 'Save changes', class: "btn btn-success" diff --git a/app/views/admin/application_settings/reporting.html.haml b/app/views/admin/application_settings/reporting.html.haml index 1c2d9ccdb2d..46e3d1c4570 100644 --- a/app/views/admin/application_settings/reporting.html.haml +++ b/app/views/admin/application_settings/reporting.html.haml @@ -23,14 +23,3 @@ = _('Set notification email for abuse reports.') .settings-content = render 'abuse' - -%section.settings.as-logging.no-animate#js-logging-settings{ class: ('expanded' if expanded_by_default?) } - .settings-header - %h4 - = _('Error Reporting and Logging') - %button.btn.btn-default.js-settings-toggle{ type: 'button' } - = expanded_by_default? ? _('Collapse') : _('Expand') - %p - = _('Enable Sentry for error reporting and logging.') - .settings-content - = render 'logging' diff --git a/app/views/layouts/_head.html.haml b/app/views/layouts/_head.html.haml index 7535aee83a3..20b844f9fd8 100644 --- a/app/views/layouts/_head.html.haml +++ b/app/views/layouts/_head.html.haml @@ -46,7 +46,7 @@ = yield :library_javascripts = javascript_include_tag locale_path unless I18n.locale == :en - = webpack_bundle_tag "raven" if Gitlab::CurrentSettings.clientside_sentry_enabled + = webpack_bundle_tag "raven" if Gitlab.config.sentry.enabled - if content_for?(:page_specific_javascripts) = yield :page_specific_javascripts diff --git a/changelogs/unreleased/refactor-sentry.yml b/changelogs/unreleased/refactor-sentry.yml new file mode 100644 index 00000000000..25c5534fae0 --- /dev/null +++ b/changelogs/unreleased/refactor-sentry.yml @@ -0,0 +1,5 @@ +--- +title: Remove Sentry from application settings +merge_request: 28447 +author: Roger Meier +type: added diff --git a/config/initializers/sentry.rb b/config/initializers/sentry.rb index e5589ce0ad1..fcc6bfa5c92 100644 --- a/config/initializers/sentry.rb +++ b/config/initializers/sentry.rb @@ -3,18 +3,11 @@ require 'gitlab/current_settings' def configure_sentry - # allow it to fail: it may do so when create_from_defaults is executed before migrations are actually done - begin - sentry_enabled = Gitlab::CurrentSettings.current_application_settings.sentry_enabled - rescue - sentry_enabled = false - end - - if sentry_enabled + if Gitlab::Sentry.enabled? Raven.configure do |config| - config.dsn = Gitlab::CurrentSettings.current_application_settings.sentry_dsn + config.dsn = Gitlab.config.sentry.dsn config.release = Gitlab.revision - config.current_environment = Gitlab.config.sentry.environment.presence + config.current_environment = Gitlab.config.sentry.environment # Sanitize fields based on those sanitized from Rails. config.sanitize_fields = Rails.application.config.filter_parameters.map(&:to_s) diff --git a/db/post_migrate/remove_sentry_from_application_settings.rb b/db/post_migrate/remove_sentry_from_application_settings.rb new file mode 100644 index 00000000000..c9fee63ad23 --- /dev/null +++ b/db/post_migrate/remove_sentry_from_application_settings.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class RemoveSentryFromApplicationSettings < ActiveRecord::Migration[5.0] + include Gitlab::Database::MigrationHelpers + + DOWNTIME = false + + SENTRY_ENABLED_COLUMNS = [ + :sentry_enabled, + :clientside_sentry_enabled + ].freeze + + SENTRY_DSN_COLUMNS = [ + :sentry_dsn, + :clientside_sentry_dsn + ].freeze + + def up + (SENTRY_ENABLED_COLUMNS + SENTRY_DSN_COLUMNS).each do |column| + remove_column(:application_settings, column) if column_exists?(:application_settings, column) + end + end + + def down + SENTRY_ENABLED_COLUMNS.each do |column| + add_column_with_default(:application_settings, column, :boolean, default: false, allow_null: false) unless column_exists?(:application_settings, column) + end + + SENTRY_DSN_COLUMNS.each do |column| + add_column(:application_settings, column, :string) unless column_exists?(:application_settings, column) + end + end +end diff --git a/doc/api/settings.md b/doc/api/settings.md index eb3f39e6670..b01cec64837 100644 --- a/doc/api/settings.md +++ b/doc/api/settings.md @@ -142,8 +142,6 @@ are listed in the descriptions of the relevant settings. | `authorized_keys_enabled` | boolean | no | By default, we write to the `authorized_keys` file to support Git over SSH without additional configuration. GitLab can be optimized to authenticate SSH keys via the database file. Only disable this if you have configured your OpenSSH server to use the AuthorizedKeysCommand. | | `auto_devops_domain` | string | no | Specify a domain to use by default for every project's Auto Review Apps and Auto Deploy stages. | | `auto_devops_enabled` | boolean | no | Enable Auto DevOps for projects by default. It will automatically build, test, and deploy applications based on a predefined CI/CD configuration. | -| `clientside_sentry_dsn` | string | required by: `clientside_sentry_enabled` | Clientside Sentry Data Source Name. | -| `clientside_sentry_enabled` | boolean | no | (**If enabled, requires:** `clientside_sentry_dsn`) Enable Sentry error reporting for the client side. | | `container_registry_token_expire_delay` | integer | no | Container Registry token duration in minutes. | | `default_artifacts_expire_in` | string | no | Set the default expiration time for each job's artifacts. | | `default_branch_protection` | integer | no | Determine if developers can push to master. Can take: `0` _(not protected, both developers and maintainers can push new commits, force push, or delete the branch)_, `1` _(partially protected, developers and maintainers can push new commits, but cannot force push or delete the branch)_ or `2` _(fully protected, developers cannot push new commits, but maintainers can; no-one can force push or delete the branch)_ as a parameter. Default is `2`. | @@ -212,8 +210,6 @@ are listed in the descriptions of the relevant settings. | `restricted_visibility_levels` | array of strings | no | Selected levels cannot be used by non-admin users for groups, projects or snippets. Can take `private`, `internal` and `public` as a parameter. Default is `null` which means there is no restriction. | | `rsa_key_restriction` | integer | no | The minimum allowed bit length of an uploaded RSA key. Default is `0` (no restriction). `-1` disables RSA keys. | | `send_user_confirmation_email` | boolean | no | Send confirmation email on sign-up. | -| `sentry_dsn` | string | required by: `sentry_enabled` | Sentry Data Source Name. | -| `sentry_enabled` | boolean | no | (**If enabled, requires:** `sentry_dsn`) Sentry is an error reporting and logging tool which is currently not shipped with GitLab, available at <https://sentry.io>. | | `session_expire_delay` | integer | no | Session duration in minutes. GitLab restart is required to apply changes | | `shared_runners_enabled` | boolean | no | (**If enabled, requires:** `shared_runners_text`) Enable shared runners for new projects. | | `shared_runners_text` | string | required by: `shared_runners_enabled` | Shared runners text. | diff --git a/lib/api/settings.rb b/lib/api/settings.rb index 6767ef882cb..3c5c1a9fd5f 100644 --- a/lib/api/settings.rb +++ b/lib/api/settings.rb @@ -36,10 +36,6 @@ module API given akismet_enabled: ->(val) { val } do requires :akismet_api_key, type: String, desc: 'Generate API key at http://www.akismet.com' end - optional :clientside_sentry_enabled, type: Boolean, desc: 'Sentry can also be used for reporting and logging clientside exceptions. https://sentry.io/for/javascript/' - given clientside_sentry_enabled: ->(val) { val } do - requires :clientside_sentry_dsn, type: String, desc: 'Clientside Sentry Data Source Name' - end optional :container_registry_token_expire_delay, type: Integer, desc: 'Authorization token duration (minutes)' optional :default_artifacts_expire_in, type: String, desc: "Set the default expiration time for each job's artifacts" optional :default_project_creation, type: Integer, values: ::Gitlab::Access.project_creation_values, desc: 'Determine if developers can create projects in the group' @@ -114,10 +110,6 @@ module API end optional :restricted_visibility_levels, type: Array[String], desc: 'Selected levels cannot be used by non-admin users for groups, projects or snippets. If the public level is restricted, user profiles are only visible to logged in users.' optional :send_user_confirmation_email, type: Boolean, desc: 'Send confirmation email on sign-up' - optional :sentry_enabled, type: Boolean, desc: 'Sentry is an error reporting and logging tool which is currently not shipped with GitLab, get it here: https://getsentry.com' - given sentry_enabled: ->(val) { val } do - requires :sentry_dsn, type: String, desc: 'Sentry Data Source Name' - end optional :session_expire_delay, type: Integer, desc: 'Session duration in minutes. GitLab restart is required to apply changes.' optional :shared_runners_enabled, type: Boolean, desc: 'Enable shared runners for new projects' given shared_runners_enabled: ->(val) { val } do diff --git a/lib/gitlab/gon_helper.rb b/lib/gitlab/gon_helper.rb index 582c3065189..92917028851 100644 --- a/lib/gitlab/gon_helper.rb +++ b/lib/gitlab/gon_helper.rb @@ -16,8 +16,8 @@ module Gitlab gon.shortcuts_path = Gitlab::Routing.url_helpers.help_page_path('shortcuts') gon.user_color_scheme = Gitlab::ColorSchemes.for_user(current_user).css_class - if Gitlab::CurrentSettings.clientside_sentry_enabled - gon.sentry_dsn = Gitlab::CurrentSettings.clientside_sentry_dsn + if Gitlab.config.sentry.enabled + gon.sentry_dsn = Gitlab.config.sentry.clientside_dsn gon.sentry_environment = Gitlab.config.sentry.environment end diff --git a/lib/gitlab/sentry.rb b/lib/gitlab/sentry.rb index 72c44114001..764db14d720 100644 --- a/lib/gitlab/sentry.rb +++ b/lib/gitlab/sentry.rb @@ -4,7 +4,7 @@ module Gitlab module Sentry def self.enabled? (Rails.env.production? || Rails.env.development?) && - Gitlab::CurrentSettings.sentry_enabled? + Gitlab.config.sentry.enabled end def self.context(current_user = nil) diff --git a/locale/gitlab.pot b/locale/gitlab.pot index fd6d3ed624c..ae35781c866 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -3796,9 +3796,6 @@ msgstr "" msgid "Enable HTML emails" msgstr "" -msgid "Enable Sentry for error reporting and logging." -msgstr "" - msgid "Enable access to the Performance Bar for a given group." msgstr "" @@ -4018,9 +4015,6 @@ msgstr "" msgid "Error" msgstr "" -msgid "Error Reporting and Logging" -msgstr "" - msgid "Error Tracking" msgstr "" diff --git a/spec/features/raven_js_spec.rb b/spec/features/raven_js_spec.rb index 9a049764dec..a4dd79b3179 100644 --- a/spec/features/raven_js_spec.rb +++ b/spec/features/raven_js_spec.rb @@ -10,7 +10,7 @@ describe 'RavenJS' do end it 'loads raven if sentry is enabled' do - stub_application_setting(clientside_sentry_dsn: 'https://key@domain.com/id', clientside_sentry_enabled: true) + stub_sentry_settings visit new_user_session_path diff --git a/spec/models/application_setting_spec.rb b/spec/models/application_setting_spec.rb index f8dc1541dd3..ab6f6dfe720 100644 --- a/spec/models/application_setting_spec.rb +++ b/spec/models/application_setting_spec.rb @@ -354,36 +354,6 @@ describe ApplicationSetting do end end - describe 'setting Sentry DSNs' do - context 'server DSN' do - it 'strips leading and trailing whitespace' do - subject.update(sentry_dsn: ' http://test ') - - expect(subject.sentry_dsn).to eq('http://test') - end - - it 'handles nil values' do - subject.update(sentry_dsn: nil) - - expect(subject.sentry_dsn).to be_nil - end - end - - context 'client-side DSN' do - it 'strips leading and trailing whitespace' do - subject.update(clientside_sentry_dsn: ' http://test ') - - expect(subject.clientside_sentry_dsn).to eq('http://test') - end - - it 'handles nil values' do - subject.update(clientside_sentry_dsn: nil) - - expect(subject.clientside_sentry_dsn).to be_nil - end - end - end - describe '#disabled_oauth_sign_in_sources=' do before do allow(Devise).to receive(:omniauth_providers).and_return([:github]) diff --git a/spec/requests/api/helpers_spec.rb b/spec/requests/api/helpers_spec.rb index ed907841bd8..1c69f5dbb67 100644 --- a/spec/requests/api/helpers_spec.rb +++ b/spec/requests/api/helpers_spec.rb @@ -226,10 +226,8 @@ describe API::Helpers do allow_any_instance_of(self.class).to receive(:rack_response) allow(Gitlab::Sentry).to receive(:enabled?).and_return(true) - stub_application_setting( - sentry_enabled: true, - sentry_dsn: "dummy://12345:67890@sentry.localdomain/sentry/42" - ) + stub_sentry_settings + configure_sentry Raven.client.configuration.encoding = 'json' end diff --git a/spec/support/helpers/stub_configuration.rb b/spec/support/helpers/stub_configuration.rb index f6c613ad5aa..0d591f038ce 100644 --- a/spec/support/helpers/stub_configuration.rb +++ b/spec/support/helpers/stub_configuration.rb @@ -81,6 +81,12 @@ module StubConfiguration allow(Gitlab.config.repositories).to receive(:storages).and_return(Settingslogic.new(messages)) end + def stub_sentry_settings + allow(Gitlab.config.sentry).to receive(:enabled).and_return(true) + allow(Gitlab.config.sentry).to receive(:dsn).and_return('dummy://b44a0828b72421a6d8e99efd68d44fa8@example.com/42') + allow(Gitlab.config.sentry).to receive(:clientside_dsn).and_return('dummy://b44a0828b72421a6d8e99efd68d44fa8@example.com/43') + end + def stub_kerberos_setting(messages) allow(Gitlab.config.kerberos).to receive_messages(to_settings(messages)) end diff --git a/spec/support/shared_examples/application_setting_examples.rb b/spec/support/shared_examples/application_setting_examples.rb index 421303c97be..e7ec24c5b7e 100644 --- a/spec/support/shared_examples/application_setting_examples.rb +++ b/spec/support/shared_examples/application_setting_examples.rb @@ -249,43 +249,4 @@ RSpec.shared_examples 'application settings examples' do expect(setting.password_authentication_enabled_for_web?).to be_falsey end - - describe 'sentry settings' do - context 'when the sentry settings are not set in gitlab.yml' do - it 'fallbacks to the settings in the database' do - setting.sentry_enabled = true - setting.sentry_dsn = 'https://b44a0828b72421a6d8e99efd68d44fa8@example.com/40' - setting.clientside_sentry_enabled = true - setting.clientside_sentry_dsn = 'https://b44a0828b72421a6d8e99efd68d44fa8@example.com/41' - - allow(Gitlab.config.sentry).to receive(:enabled).and_return(false) - allow(Gitlab.config.sentry).to receive(:dsn).and_return(nil) - allow(Gitlab.config.sentry).to receive(:clientside_dsn).and_return(nil) - - expect(setting.sentry_enabled).to eq true - expect(setting.sentry_dsn).to eq 'https://b44a0828b72421a6d8e99efd68d44fa8@example.com/40' - expect(setting.clientside_sentry_enabled).to eq true - expect(setting.clientside_sentry_dsn). to eq 'https://b44a0828b72421a6d8e99efd68d44fa8@example.com/41' - end - end - - context 'when the sentry settings are set in gitlab.yml' do - it 'does not fallback to the settings in the database' do - setting.sentry_enabled = false - setting.sentry_dsn = 'https://b44a0828b72421a6d8e99efd68d44fa8@example.com/40' - setting.clientside_sentry_enabled = false - setting.clientside_sentry_dsn = 'https://b44a0828b72421a6d8e99efd68d44fa8@example.com/41' - - allow(Gitlab.config.sentry).to receive(:enabled).and_return(true) - allow(Gitlab.config.sentry).to receive(:dsn).and_return('https://b44a0828b72421a6d8e99efd68d44fa8@example.com/42') - allow(Gitlab.config.sentry).to receive(:clientside_dsn).and_return('https://b44a0828b72421a6d8e99efd68d44fa8@example.com/43') - - expect(setting).not_to receive(:read_attribute) - expect(setting.sentry_enabled).to eq true - expect(setting.sentry_dsn).to eq 'https://b44a0828b72421a6d8e99efd68d44fa8@example.com/42' - expect(setting.clientside_sentry_enabled).to eq true - expect(setting.clientside_sentry_dsn). to eq 'https://b44a0828b72421a6d8e99efd68d44fa8@example.com/43' - end - end - end end |