summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
Diffstat
-rw-r--r--app/models/application_setting.rb2
-rw-r--r--app/models/concerns/cacheable_attributes.rb9
-rw-r--r--app/services/self_monitoring/project/create_service.rb105
-rw-r--r--db/migrate/20190724112147_add_column_for_self_monitoring_project_id.rb14
-rw-r--r--db/schema.rb3
-rw-r--r--lib/gitlab/current_settings.rb5
-rw-r--r--locale/gitlab.pot21
-rw-r--r--spec/lib/gitlab/current_settings_spec.rb10
-rw-r--r--spec/models/concerns/cacheable_attributes_spec.rb45
-rw-r--r--spec/services/self_monitoring/project/create_service_spec.rb71
10 files changed, 233 insertions, 52 deletions
diff --git a/app/models/application_setting.rb b/app/models/application_setting.rb
index 9dbcef8abaa..cb6346421ec 100644
--- a/app/models/application_setting.rb
+++ b/app/models/application_setting.rb
@@ -10,6 +10,8 @@ class ApplicationSetting < ApplicationRecord
add_authentication_token_field :runners_registration_token, encrypted: -> { Feature.enabled?(:application_settings_tokens_optional_encryption, default_enabled: true) ? :optional : :required }
add_authentication_token_field :health_check_access_token
+ belongs_to :instance_administration_project, class_name: "Project"
+
# Include here so it can override methods from
# `add_authentication_token_field`
# We don't prepend for now because otherwise we'll need to
diff --git a/app/models/concerns/cacheable_attributes.rb b/app/models/concerns/cacheable_attributes.rb
index 53dff2adfc3..0c800621a55 100644
--- a/app/models/concerns/cacheable_attributes.rb
+++ b/app/models/concerns/cacheable_attributes.rb
@@ -5,6 +5,8 @@ module CacheableAttributes
included do
after_commit { self.class.expire }
+
+ private_class_method :request_store_cache_key
end
class_methods do
@@ -32,7 +34,11 @@ module CacheableAttributes
end
def cached
- Gitlab::SafeRequestStore[:"#{name}_cached_attributes"] ||= retrieve_from_cache
+ Gitlab::SafeRequestStore[request_store_cache_key] ||= retrieve_from_cache
+ end
+
+ def request_store_cache_key
+ :"#{name}_cached_attributes"
end
def retrieve_from_cache
@@ -58,6 +64,7 @@ module CacheableAttributes
end
def expire
+ Gitlab::SafeRequestStore.delete(request_store_cache_key)
cache_backend.delete(cache_key)
rescue
# Gracefully handle when Redis is not available. For example,
diff --git a/app/services/self_monitoring/project/create_service.rb b/app/services/self_monitoring/project/create_service.rb
index 8ffd22de127..c925c6a1610 100644
--- a/app/services/self_monitoring/project/create_service.rb
+++ b/app/services/self_monitoring/project/create_service.rb
@@ -4,16 +4,22 @@ module SelfMonitoring
module Project
class CreateService < ::BaseService
include Stepable
+ include Gitlab::Utils::StrongMemoize
- DEFAULT_VISIBILITY_LEVEL = Gitlab::VisibilityLevel::INTERNAL
- DEFAULT_NAME = 'GitLab Instance Administration'
- DEFAULT_DESCRIPTION = <<~HEREDOC
+ VISIBILITY_LEVEL = Gitlab::VisibilityLevel::INTERNAL
+ PROJECT_NAME = 'GitLab Instance Administration'
+ PROJECT_DESCRIPTION = <<~HEREDOC
This project is automatically generated and will be used to help monitor this GitLab instance.
HEREDOC
+ GROUP_NAME = 'GitLab Instance Administrators'
+ GROUP_PATH = 'gitlab-instance-administrators'
+
steps :validate_admins,
+ :create_group,
:create_project,
- :add_project_members,
+ :save_project_id,
+ :add_group_members,
:add_to_whitelist,
:add_prometheus_manual_configuration
@@ -36,20 +42,60 @@ module SelfMonitoring
success
end
+ def create_group
+ if project_created?
+ log_info(_('Instance administrators group already exists'))
+ @group = application_settings.instance_administration_project.owner
+ return success(group: @group)
+ end
+
+ admin_user = group_owner
+ @group = ::Groups::CreateService.new(admin_user, create_group_params).execute
+
+ if @group.persisted?
+ success(group: @group)
+ else
+ error('Could not create group')
+ end
+ end
+
def create_project
- admin_user = project_owner
+ if project_created?
+ log_info(_('Instance administration project already exists'))
+ @project = application_settings.instance_administration_project
+ return success(project: project)
+ end
+
+ admin_user = group_owner
@project = ::Projects::CreateService.new(admin_user, create_project_params).execute
if project.persisted?
success(project: project)
else
- log_error("Could not create self-monitoring project. Errors: #{project.errors.full_messages}")
- error('Could not create project')
+ log_error(_("Could not create instance administration project. Errors: %{errors}") % { errors: project.errors.full_messages })
+ error(_('Could not create project'))
end
end
- def add_project_members
- members = project.add_users(project_maintainers, Gitlab::Access::MAINTAINER)
+ def save_project_id
+ return success if project_created?
+
+ result = ApplicationSettings::UpdateService.new(
+ application_settings,
+ group_owner,
+ { instance_administration_project_id: @project.id }
+ ).execute
+
+ if result
+ success
+ else
+ log_error(_("Could not save instance administration project ID, errors: %{errors}") % { errors: application_settings.errors.full_messages })
+ error(_('Could not save project ID'))
+ end
+ end
+
+ def add_group_members
+ members = @group.add_users(group_maintainers, Gitlab::Access::MAINTAINER)
errors = members.flat_map { |member| member.errors.full_messages }
if errors.any?
@@ -68,14 +114,15 @@ module SelfMonitoring
return error(_('Prometheus listen_address is not a valid URI')) unless uri
result = ApplicationSettings::UpdateService.new(
- Gitlab::CurrentSettings.current_application_settings,
- project_owner,
- outbound_local_requests_whitelist: [uri.normalized_host]
+ application_settings,
+ group_owner,
+ add_to_outbound_local_requests_whitelist: [uri.normalized_host]
).execute
if result
success
else
+ log_error(_("Could not add prometheus URL to whitelist, errors: %{errors}") % { errors: application_settings.errors.full_messages })
error(_('Could not add prometheus URL to whitelist'))
end
end
@@ -94,6 +141,17 @@ module SelfMonitoring
success
end
+ def application_settings
+ strong_memoize(:application_settings) do
+ Gitlab::CurrentSettings.expire_current_application_settings
+ Gitlab::CurrentSettings.current_application_settings
+ end
+ end
+
+ def project_created?
+ application_settings.instance_administration_project.present?
+ end
+
def parse_url(uri_string)
Addressable::URI.parse(uri_string)
rescue Addressable::URI::InvalidURIError, TypeError
@@ -114,21 +172,30 @@ module SelfMonitoring
@instance_admins ||= User.admins.active
end
- def project_owner
+ def group_owner
instance_admins.first
end
- def project_maintainers
- # Exclude the first so that the project_owner is not added again as a member.
- instance_admins - [project_owner]
+ def group_maintainers
+ # Exclude the first so that the group_owner is not added again as a member.
+ instance_admins - [group_owner]
+ end
+
+ def create_group_params
+ {
+ name: GROUP_NAME,
+ path: "#{GROUP_PATH}-#{SecureRandom.hex(4)}",
+ visibility_level: VISIBILITY_LEVEL
+ }
end
def create_project_params
{
initialize_with_readme: true,
- visibility_level: DEFAULT_VISIBILITY_LEVEL,
- name: DEFAULT_NAME,
- description: DEFAULT_DESCRIPTION
+ visibility_level: VISIBILITY_LEVEL,
+ name: PROJECT_NAME,
+ description: PROJECT_DESCRIPTION,
+ namespace_id: @group.id
}
end
diff --git a/db/migrate/20190724112147_add_column_for_self_monitoring_project_id.rb b/db/migrate/20190724112147_add_column_for_self_monitoring_project_id.rb
new file mode 100644
index 00000000000..ce249a527e6
--- /dev/null
+++ b/db/migrate/20190724112147_add_column_for_self_monitoring_project_id.rb
@@ -0,0 +1,14 @@
+# frozen_string_literal: true
+
+class AddColumnForSelfMonitoringProjectId < ActiveRecord::Migration[5.2]
+ DOWNTIME = false
+
+ def change
+ add_reference(
+ :application_settings,
+ :instance_administration_project,
+ index: { name: 'index_applicationsettings_on_instance_administration_project_id' },
+ foreign_key: { to_table: :projects, on_delete: :nullify }
+ )
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 2fac2c01e00..890f4ab4d8a 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -230,9 +230,11 @@ ActiveRecord::Schema.define(version: 2019_08_02_235445) do
t.integer "raw_blob_request_limit", default: 300, null: false
t.boolean "allow_local_requests_from_web_hooks_and_services", default: false, null: false
t.boolean "allow_local_requests_from_system_hooks", default: true, null: false
+ t.bigint "instance_administration_project_id"
t.string "snowplow_collector_hostname"
t.index ["custom_project_templates_group_id"], name: "index_application_settings_on_custom_project_templates_group_id"
t.index ["file_template_project_id"], name: "index_application_settings_on_file_template_project_id"
+ t.index ["instance_administration_project_id"], name: "index_applicationsettings_on_instance_administration_project_id"
t.index ["usage_stats_set_by_user_id"], name: "index_application_settings_on_usage_stats_set_by_user_id"
end
@@ -3623,6 +3625,7 @@ ActiveRecord::Schema.define(version: 2019_08_02_235445) do
add_foreign_key "application_settings", "namespaces", column: "custom_project_templates_group_id", on_delete: :nullify
add_foreign_key "application_settings", "projects", column: "file_template_project_id", name: "fk_ec757bd087", on_delete: :nullify
+ add_foreign_key "application_settings", "projects", column: "instance_administration_project_id", on_delete: :nullify
add_foreign_key "application_settings", "users", column: "usage_stats_set_by_user_id", name: "fk_964370041d", on_delete: :nullify
add_foreign_key "approval_merge_request_rule_sources", "approval_merge_request_rules", on_delete: :cascade
add_foreign_key "approval_merge_request_rule_sources", "approval_project_rules", on_delete: :cascade
diff --git a/lib/gitlab/current_settings.rb b/lib/gitlab/current_settings.rb
index 32d5e4b9ea3..6ce47650562 100644
--- a/lib/gitlab/current_settings.rb
+++ b/lib/gitlab/current_settings.rb
@@ -7,6 +7,11 @@ module Gitlab
Gitlab::SafeRequestStore.fetch(:current_application_settings) { ensure_application_settings! }
end
+ def expire_current_application_settings
+ ::ApplicationSetting.expire
+ Gitlab::SafeRequestStore.delete(:current_application_settings)
+ end
+
def clear_in_memory_application_settings!
@in_memory_application_settings = nil
end
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 022d1745a1b..7586fff2e7d 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -3297,6 +3297,9 @@ msgstr ""
msgid "Could not add prometheus URL to whitelist"
msgstr ""
+msgid "Could not add prometheus URL to whitelist, errors: %{errors}"
+msgstr ""
+
msgid "Could not authorize chat nickname. Try again!"
msgstr ""
@@ -3309,6 +3312,12 @@ msgstr ""
msgid "Could not create Wiki Repository at this time. Please try again later."
msgstr ""
+msgid "Could not create instance administration project. Errors: %{errors}"
+msgstr ""
+
+msgid "Could not create project"
+msgstr ""
+
msgid "Could not delete chat nickname %{chat_name}."
msgstr ""
@@ -3324,6 +3333,12 @@ msgstr ""
msgid "Could not revoke personal access token %{personal_access_token_name}."
msgstr ""
+msgid "Could not save instance administration project ID, errors: %{errors}"
+msgstr ""
+
+msgid "Could not save project ID"
+msgstr ""
+
msgid "Coverage"
msgstr ""
@@ -5787,6 +5802,12 @@ msgstr ""
msgid "Instance Statistics visibility"
msgstr ""
+msgid "Instance administration project already exists"
+msgstr ""
+
+msgid "Instance administrators group already exists"
+msgstr ""
+
msgid "Instance does not support multiple Kubernetes clusters"
msgstr ""
diff --git a/spec/lib/gitlab/current_settings_spec.rb b/spec/lib/gitlab/current_settings_spec.rb
index 2759412add8..eced96a4c77 100644
--- a/spec/lib/gitlab/current_settings_spec.rb
+++ b/spec/lib/gitlab/current_settings_spec.rb
@@ -14,6 +14,16 @@ describe Gitlab::CurrentSettings do
end
end
+ describe '.expire_current_application_settings', :use_clean_rails_memory_store_caching, :request_store do
+ include_context 'with settings in cache'
+
+ it 'expires the cache' do
+ described_class.expire_current_application_settings
+
+ expect(ActiveRecord::QueryRecorder.new { described_class.current_application_settings }.count).not_to eq(0)
+ end
+ end
+
describe '#current_application_settings', :use_clean_rails_memory_store_caching do
it 'allows keys to be called directly' do
db_settings = create(:application_setting,
diff --git a/spec/models/concerns/cacheable_attributes_spec.rb b/spec/models/concerns/cacheable_attributes_spec.rb
index eeacdadab9c..da46effe411 100644
--- a/spec/models/concerns/cacheable_attributes_spec.rb
+++ b/spec/models/concerns/cacheable_attributes_spec.rb
@@ -7,6 +7,7 @@ describe CacheableAttributes do
Class.new do
include ActiveModel::Model
extend ActiveModel::Callbacks
+ include ActiveModel::AttributeMethods
define_model_callbacks :commit
include CacheableAttributes
@@ -34,44 +35,60 @@ describe CacheableAttributes do
end
end
+ before do
+ stub_const("MinimalTestClass", minimal_test_class)
+ end
+
shared_context 'with defaults' do
before do
- minimal_test_class.define_singleton_method(:defaults) do
+ MinimalTestClass.define_singleton_method(:defaults) do
{ foo: 'a', bar: 'b', baz: 'c' }
end
end
end
+ describe '.expire', :use_clean_rails_memory_store_caching, :request_store do
+ it 'wipes the cache' do
+ obj = MinimalTestClass.new
+ obj.cache!
+ expect(MinimalTestClass.cached).not_to eq(nil)
+
+ MinimalTestClass.expire
+
+ expect(MinimalTestClass.cached).to eq(nil)
+ end
+ end
+
describe '.current_without_cache' do
it 'defaults to last' do
- expect(minimal_test_class.current_without_cache).to eq(minimal_test_class.last)
+ expect(MinimalTestClass.current_without_cache).to eq(MinimalTestClass.last)
end
it 'can be overridden' do
- minimal_test_class.define_singleton_method(:current_without_cache) do
+ MinimalTestClass.define_singleton_method(:current_without_cache) do
first
end
- expect(minimal_test_class.current_without_cache).to eq(minimal_test_class.first)
+ expect(MinimalTestClass.current_without_cache).to eq(MinimalTestClass.first)
end
end
describe '.cache_key' do
it 'excludes cache attributes' do
- expect(minimal_test_class.cache_key).to eq("TestClass:#{Gitlab::VERSION}:#{Rails.version}")
+ expect(MinimalTestClass.cache_key).to eq("TestClass:#{Gitlab::VERSION}:#{Rails.version}")
end
end
describe '.defaults' do
it 'defaults to {}' do
- expect(minimal_test_class.defaults).to eq({})
+ expect(MinimalTestClass.defaults).to eq({})
end
context 'with defaults defined' do
include_context 'with defaults'
it 'can be overridden' do
- expect(minimal_test_class.defaults).to eq({ foo: 'a', bar: 'b', baz: 'c' })
+ expect(MinimalTestClass.defaults).to eq({ foo: 'a', bar: 'b', baz: 'c' })
end
end
end
@@ -81,13 +98,13 @@ describe CacheableAttributes do
context 'without any attributes given' do
it 'intializes a new object with the defaults' do
- expect(minimal_test_class.build_from_defaults.attributes).to eq(minimal_test_class.defaults.stringify_keys)
+ expect(MinimalTestClass.build_from_defaults.attributes).to eq(MinimalTestClass.defaults.stringify_keys)
end
end
context 'with attributes given' do
it 'intializes a new object with the given attributes merged into the defaults' do
- expect(minimal_test_class.build_from_defaults(foo: 'd').attributes['foo']).to eq('d')
+ expect(MinimalTestClass.build_from_defaults(foo: 'd').attributes['foo']).to eq('d')
end
end
@@ -108,8 +125,8 @@ describe CacheableAttributes do
describe '.current', :use_clean_rails_memory_store_caching do
context 'redis unavailable' do
before do
- allow(minimal_test_class).to receive(:last).and_return(:last)
- expect(Rails.cache).to receive(:read).with(minimal_test_class.cache_key).and_raise(Redis::BaseError)
+ allow(MinimalTestClass).to receive(:last).and_return(:last)
+ expect(Rails.cache).to receive(:read).with(MinimalTestClass.cache_key).and_raise(Redis::BaseError)
end
context 'in production environment' do
@@ -120,7 +137,7 @@ describe CacheableAttributes do
it 'returns an uncached record and logs a warning' do
expect(Rails.logger).to receive(:warn).with("Cached record for TestClass couldn't be loaded, falling back to uncached record: Redis::BaseError")
- expect(minimal_test_class.current).to eq(:last)
+ expect(MinimalTestClass.current).to eq(:last)
end
end
@@ -132,7 +149,7 @@ describe CacheableAttributes do
it 'returns an uncached record and logs a warning' do
expect(Rails.logger).not_to receive(:warn)
- expect { minimal_test_class.current }.to raise_error(Redis::BaseError)
+ expect { MinimalTestClass.current }.to raise_error(Redis::BaseError)
end
end
end
@@ -202,7 +219,7 @@ describe CacheableAttributes do
describe '.cached', :use_clean_rails_memory_store_caching do
context 'when cache is cold' do
it 'returns nil' do
- expect(minimal_test_class.cached).to be_nil
+ expect(MinimalTestClass.cached).to be_nil
end
end
diff --git a/spec/services/self_monitoring/project/create_service_spec.rb b/spec/services/self_monitoring/project/create_service_spec.rb
index 7d4faba526b..def20448bd9 100644
--- a/spec/services/self_monitoring/project/create_service_spec.rb
+++ b/spec/services/self_monitoring/project/create_service_spec.rb
@@ -30,15 +30,13 @@ describe SelfMonitoring::Project::CreateService do
context 'with admin users' do
let(:project) { result[:project] }
+ let(:group) { result[:group] }
+ let(:application_setting) { Gitlab::CurrentSettings.current_application_settings }
let!(:user) { create(:user, :admin) }
before do
- allow(ApplicationSetting)
- .to receive(:current)
- .and_return(
- ApplicationSetting.build_from_defaults(allow_local_requests_from_web_hooks_and_services: true)
- )
+ application_setting.allow_local_requests_from_web_hooks_and_services = true
end
shared_examples 'has prometheus service' do |listen_address|
@@ -55,6 +53,15 @@ describe SelfMonitoring::Project::CreateService do
it_behaves_like 'has prometheus service', 'http://localhost:9090'
+ it 'creates group' do
+ expect(result[:status]).to eq(:success)
+ expect(group).to be_persisted
+ expect(group.name).to eq(described_class::GROUP_NAME)
+ expect(group.path).to start_with(described_class::GROUP_PATH)
+ expect(group.path.split('-').last.length).to eq(8)
+ expect(group.visibility_level).to eq(described_class::VISIBILITY_LEVEL)
+ end
+
it 'creates project with internal visibility' do
expect(result[:status]).to eq(:success)
expect(project.visibility_level).to eq(Gitlab::VisibilityLevel::INTERNAL)
@@ -62,7 +69,7 @@ describe SelfMonitoring::Project::CreateService do
end
it 'creates project with internal visibility even when internal visibility is restricted' do
- stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::INTERNAL])
+ application_setting.restricted_visibility_levels = [Gitlab::VisibilityLevel::INTERNAL]
expect(result[:status]).to eq(:success)
expect(project.visibility_level).to eq(Gitlab::VisibilityLevel::INTERNAL)
@@ -71,8 +78,8 @@ describe SelfMonitoring::Project::CreateService do
it 'creates project with correct name and description' do
expect(result[:status]).to eq(:success)
- expect(project.name).to eq(described_class::DEFAULT_NAME)
- expect(project.description).to eq(described_class::DEFAULT_DESCRIPTION)
+ expect(project.name).to eq(described_class::PROJECT_NAME)
+ expect(project.description).to eq(described_class::PROJECT_DESCRIPTION)
end
it 'adds all admins as maintainers' do
@@ -81,25 +88,53 @@ describe SelfMonitoring::Project::CreateService do
create(:user)
expect(result[:status]).to eq(:success)
- expect(project.owner).to eq(user)
- expect(project.members.collect(&:user)).to contain_exactly(user, admin1, admin2)
- expect(project.members.collect(&:access_level)).to contain_exactly(
- Gitlab::Access::MAINTAINER,
+ expect(project.owner).to eq(group)
+ expect(group.members.collect(&:user)).to contain_exactly(user, admin1, admin2)
+ expect(group.members.collect(&:access_level)).to contain_exactly(
+ Gitlab::Access::OWNER,
Gitlab::Access::MAINTAINER,
Gitlab::Access::MAINTAINER
)
end
+ it 'saves the project id' do
+ expect(result[:status]).to eq(:success)
+ expect(application_setting.instance_administration_project_id).to eq(project.id)
+ end
+
+ it 'returns error when saving project ID fails' do
+ allow(application_setting).to receive(:update) { false }
+
+ expect(result[:status]).to eq(:error)
+ expect(result[:failed_step]).to eq(:save_project_id)
+ expect(result[:message]).to eq('Could not save project ID')
+ end
+
+ it 'does not fail when a project already exists' do
+ expect(result[:status]).to eq(:success)
+
+ second_result = subject.execute
+
+ expect(second_result[:status]).to eq(:success)
+ expect(second_result[:project]).to eq(project)
+ expect(second_result[:group]).to eq(group)
+ end
+
context 'when local requests from hooks and services are not allowed' do
before do
- allow(ApplicationSetting)
- .to receive(:current)
- .and_return(
- ApplicationSetting.build_from_defaults(allow_local_requests_from_web_hooks_and_services: false)
- )
+ application_setting.allow_local_requests_from_web_hooks_and_services = false
end
it_behaves_like 'has prometheus service', 'http://localhost:9090'
+
+ it 'does not overwrite the existing whitelist' do
+ application_setting.outbound_local_requests_whitelist = ['example.com']
+
+ expect(result[:status]).to eq(:success)
+ expect(application_setting.outbound_local_requests_whitelist).to contain_exactly(
+ 'example.com', 'localhost'
+ )
+ end
end
context 'with non default prometheus address' do
@@ -175,7 +210,7 @@ describe SelfMonitoring::Project::CreateService do
expect(result).to eq({
status: :error,
message: 'Could not add admins as members',
- failed_step: :add_project_members
+ failed_step: :add_group_members
})
end
end
View Lorry Controller Status