diff options
-rw-r--r-- | app/controllers/application_controller.rb | 17 | ||||
-rw-r--r-- | app/controllers/concerns/with_performance_bar.rb | 17 | ||||
-rw-r--r-- | config/gitlab.yml.example | 5 | ||||
-rw-r--r-- | config/initializers/1_settings.rb | 6 | ||||
-rw-r--r-- | lib/feature.rb | 10 | ||||
-rw-r--r-- | lib/gitlab/performance_bar.rb | 20 | ||||
-rw-r--r-- | spec/lib/gitlab/performance_bar_spec.rb | 91 | ||||
-rw-r--r-- | spec/support/stub_configuration.rb | 4 |
8 files changed, 151 insertions, 19 deletions
diff --git a/app/controllers/application_controller.rb b/app/controllers/application_controller.rb index b4c0cd0487f..db7edbd619b 100644 --- a/app/controllers/application_controller.rb +++ b/app/controllers/application_controller.rb @@ -9,7 +9,7 @@ class ApplicationController < ActionController::Base include SentryHelper include WorkhorseHelper include EnforcesTwoFactorAuthentication - include Peek::Rblineprof::CustomControllerHelpers + include WithPerformanceBar before_action :authenticate_user_from_private_token! before_action :authenticate_user_from_rss_token! @@ -68,21 +68,6 @@ class ApplicationController < ActionController::Base end end - def peek_enabled? - return false unless Gitlab::PerformanceBar.enabled? - return false unless current_user - - if RequestStore.active? - if RequestStore.store.key?(:peek_enabled) - RequestStore.store[:peek_enabled] - else - RequestStore.store[:peek_enabled] = cookies[:perf_bar_enabled].present? - end - else - cookies[:perf_bar_enabled].present? - end - end - protected # This filter handles both private tokens and personal access tokens diff --git a/app/controllers/concerns/with_performance_bar.rb b/app/controllers/concerns/with_performance_bar.rb new file mode 100644 index 00000000000..ed253042701 --- /dev/null +++ b/app/controllers/concerns/with_performance_bar.rb @@ -0,0 +1,17 @@ +module WithPerformanceBar + extend ActiveSupport::Concern + + included do + include Peek::Rblineprof::CustomControllerHelpers + end + + def peek_enabled? + return false unless Gitlab::PerformanceBar.enabled?(current_user) + + if RequestStore.active? + RequestStore.fetch(:peek_enabled) { cookies[:perf_bar_enabled].present? } + else + cookies[:perf_bar_enabled].present? + end + end +end diff --git a/config/gitlab.yml.example b/config/gitlab.yml.example index 4b81fd90f59..e3d1141987e 100644 --- a/config/gitlab.yml.example +++ b/config/gitlab.yml.example @@ -459,6 +459,11 @@ production: &base # is the normal way to deploy Gitaly. token: + # Performance bar settings + performance_bar: + # This setting controls what group can see the performance bar. + # allowed_group: performance-group + # # 4. Advanced settings # ========================== diff --git a/config/initializers/1_settings.rb b/config/initializers/1_settings.rb index cb11d2c34f4..7d35404119f 100644 --- a/config/initializers/1_settings.rb +++ b/config/initializers/1_settings.rb @@ -486,6 +486,12 @@ Settings['gitaly'] ||= Settingslogic.new({}) Settings.gitaly['enabled'] = true if Settings.gitaly['enabled'].nil? # +# Performance bar +# +Settings['performance_bar'] ||= Settingslogic.new({}) +Settings.performance_bar['allowed_group'] = 'gitlab-org' if Settings.performance_bar['allowed_group'].nil? + +# # Webpack settings # Settings['webpack'] ||= Settingslogic.new({}) diff --git a/lib/feature.rb b/lib/feature.rb index 363f66ba60e..853a00c75a4 100644 --- a/lib/feature.rb +++ b/lib/feature.rb @@ -54,7 +54,15 @@ class Feature adapter = Flipper::Adapters::ActiveRecord.new( feature_class: FlipperFeature, gate_class: FlipperGate) - Flipper.new(adapter) + Flipper.new(adapter).tap do + register_feature_groups + end + end + end + + def register_feature_groups + Flipper.register(:performance_team) do |actor| + Gitlab::PerformanceBar.allowed_actor?(actor) end end end diff --git a/lib/gitlab/performance_bar.rb b/lib/gitlab/performance_bar.rb index 163a40ad306..2ca2cbeac08 100644 --- a/lib/gitlab/performance_bar.rb +++ b/lib/gitlab/performance_bar.rb @@ -1,7 +1,23 @@ module Gitlab module PerformanceBar - def self.enabled? - Feature.enabled?('gitlab_performance_bar') + def self.enabled?(current_user = nil) + Feature.enabled?(:gitlab_performance_bar, current_user) + end + + def self.allowed_actor?(actor) + group = allowed_group + return false unless actor&.is_a?(User) && group + + GroupMembersFinder.new(group) + .execute + .where(user_id: actor.id) + .any? + end + + def self.allowed_group + return nil unless Gitlab.config.performance_bar.allowed_group + + Group.by_path(Gitlab.config.performance_bar.allowed_group) end end end diff --git a/spec/lib/gitlab/performance_bar_spec.rb b/spec/lib/gitlab/performance_bar_spec.rb new file mode 100644 index 00000000000..0f630c243ad --- /dev/null +++ b/spec/lib/gitlab/performance_bar_spec.rb @@ -0,0 +1,91 @@ +require 'spec_helper' + +describe Gitlab::PerformanceBar do + describe '.enabled?' do + it 'returns false when given user is nil' do + expect(described_class.enabled?(nil)).to be_falsy + end + + it 'returns false when feature is disabled' do + user = double('user') + + expect(Feature).to receive(:enabled?) + .with(:gitlab_performance_bar, user).and_return(false) + + expect(described_class.enabled?(user)).to be_falsy + end + + it 'returns true when feature is enabled' do + user = double('user') + + expect(Feature).to receive(:enabled?) + .with(:gitlab_performance_bar, user).and_return(true) + + expect(described_class.enabled?(user)).to be_truthy + end + end + + describe '.allowed_actor?' do + it 'returns false when given actor is not a User' do + actor = double + + expect(described_class.allowed_actor?(actor)).to be_falsy + end + + context 'when given actor is a User' do + let(:actor) { create(:user) } + + before do + stub_performance_bar_setting(allowed_group: 'my-group') + end + + context 'when allowed group does not exist' do + it 'returns false' do + expect(described_class.allowed_actor?(actor)).to be_falsy + end + end + + context 'when allowed group exists' do + let!(:my_group) { create(:group, path: 'my-group') } + + context 'when user is not a member of the allowed group' do + it 'returns false' do + expect(described_class.allowed_actor?(actor)).to be_falsy + end + end + + context 'when user is a member of the allowed group' do + before do + my_group.add_developer(actor) + end + + it 'returns true' do + expect(described_class.allowed_actor?(actor)).to be_truthy + end + end + end + end + end + + describe '.allowed_group' do + before do + stub_performance_bar_setting(allowed_group: 'my-group') + end + + context 'when allowed group does not exist' do + it 'returns false' do + expect(described_class.allowed_group).to be_falsy + end + end + + context 'when allowed group exists' do + let!(:my_group) { create(:group, path: 'my-group') } + + context 'when user is not a member of the allowed group' do + it 'returns false' do + expect(described_class.allowed_group).to eq(my_group) + end + end + end + end +end diff --git a/spec/support/stub_configuration.rb b/spec/support/stub_configuration.rb index 48f454c7187..1d9946b0ed1 100644 --- a/spec/support/stub_configuration.rb +++ b/spec/support/stub_configuration.rb @@ -29,6 +29,10 @@ module StubConfiguration allow(Gitlab.config.omniauth).to receive_messages(messages) end + def stub_performance_bar_setting(messages) + allow(Gitlab.config.performance_bar).to receive_messages(messages) + end + private # Modifies stubbed messages to also stub possible predicate versions |