Don't use transactions and exceptions

Instead return error objects.
This commit is contained in:
Reuben Pereira 2019-07-16 19:20:43 +00:00 committed by Stan Hu
parent 58d220bd66
commit 0d84010d1c
5 changed files with 492 additions and 0 deletions

View file

@ -0,0 +1,35 @@
# frozen_string_literal: true
module Stepable
extend ActiveSupport::Concern
def steps
self.class._all_steps
end
def execute_steps
initial_result = {}
steps.inject(initial_result) do |previous_result, callback|
result = method(callback).call
if result[:status] == :error
result[:failed_step] = callback
break result
end
previous_result.merge(result)
end
end
class_methods do
def _all_steps
@_all_steps ||= []
end
def steps(*methods)
_all_steps.concat methods
end
end
end

View file

@ -0,0 +1,132 @@
# frozen_string_literal: true
module SelfMonitoring
module Project
class CreateService < ::BaseService
include Stepable
DEFAULT_VISIBILITY_LEVEL = Gitlab::VisibilityLevel::INTERNAL
DEFAULT_NAME = 'GitLab Instance Administration'
DEFAULT_DESCRIPTION = <<~HEREDOC
This project is automatically generated and will be used to help monitor this GitLab instance.
HEREDOC
steps :validate_admins,
:create_project,
:add_project_members,
:add_prometheus_manual_configuration
def initialize
super(nil)
end
def execute
execute_steps
end
private
def validate_admins
unless instance_admins.any?
log_error('No active admin user found')
return error('No active admin user found')
end
success
end
def create_project
admin_user = project_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')
end
end
def add_project_members
members = project.add_users(project_maintainers, Gitlab::Access::MAINTAINER)
errors = members.flat_map { |member| member.errors.full_messages }
if errors.any?
log_error("Could not add admins as members to self-monitoring project. Errors: #{errors}")
error('Could not add admins as members')
else
success
end
end
def add_prometheus_manual_configuration
return success unless prometheus_enabled?
return success unless prometheus_listen_address.present?
# TODO: Currently, adding the internal prometheus server as a manual configuration
# is only possible if the setting to allow webhooks and services to connect
# to local network is on.
# https://gitlab.com/gitlab-org/gitlab-ce/issues/44496 will add
# a whitelist that will allow connections to certain ips on the local network.
service = project.find_or_initialize_service('prometheus')
unless service.update(prometheus_service_attributes)
log_error("Could not save prometheus manual configuration for self-monitoring project. Errors: #{service.errors.full_messages}")
return error('Could not save prometheus manual configuration')
end
success
end
def prometheus_enabled?
Gitlab.config.prometheus.enable
rescue Settingslogic::MissingSetting
false
end
def prometheus_listen_address
Gitlab.config.prometheus.listen_address
rescue Settingslogic::MissingSetting
end
def instance_admins
@instance_admins ||= User.admins.active
end
def project_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]
end
def create_project_params
{
initialize_with_readme: true,
visibility_level: DEFAULT_VISIBILITY_LEVEL,
name: DEFAULT_NAME,
description: DEFAULT_DESCRIPTION
}
end
def internal_prometheus_listen_address_uri
if prometheus_listen_address.starts_with?('http')
prometheus_listen_address
else
'http://' + prometheus_listen_address
end
end
def prometheus_service_attributes
{
api_url: internal_prometheus_listen_address_uri,
manual_configuration: true,
active: true
}
end
end
end
end

View file

@ -952,6 +952,16 @@ production: &base
# address: localhost
# port: 3807
## Prometheus settings
# Do not modify these settings here. They should be modified in /etc/gitlab/gitlab.rb
# if you installed GitLab via Omnibus.
# If you installed from source, you need to install and configure Prometheus
# yourself, and then update the values here.
# https://docs.gitlab.com/ee/administration/monitoring/prometheus/
prometheus:
# enable: true
# listen_address: 'localhost:9090'
#
# 5. Extra customization
# ==========================
@ -1158,6 +1168,9 @@ test:
user_filter: ''
group_base: 'ou=groups,dc=example,dc=com'
admin_group: ''
prometheus:
enable: true
listen_address: 'localhost:9090'
staging:
<<: *base

View file

@ -0,0 +1,111 @@
# frozen_string_literal: true
require 'spec_helper'
describe Stepable do
let(:described_class) do
Class.new do
include Stepable
steps :method1, :method2, :method3
def execute
execute_steps
end
private
def method1
{ status: :success }
end
def method2
return { status: :error } unless @pass
{ status: :success, variable1: 'var1' }
end
def method3
{ status: :success, variable2: 'var2' }
end
end
end
let(:prepended_module) do
Module.new do
extend ActiveSupport::Concern
prepended do
steps :appended_method1
end
private
def appended_method1
{ status: :success }
end
end
end
before do
described_class.prepend(prepended_module)
end
it 'stops after the first error' do
expect(subject).not_to receive(:method3)
expect(subject).not_to receive(:appended_method1)
expect(subject.execute).to eq(
status: :error,
failed_step: :method2
)
end
context 'when all methods return success' do
before do
subject.instance_variable_set(:@pass, true)
end
it 'calls all methods in order' do
expect(subject).to receive(:method1).and_call_original.ordered
expect(subject).to receive(:method2).and_call_original.ordered
expect(subject).to receive(:method3).and_call_original.ordered
expect(subject).to receive(:appended_method1).and_call_original.ordered
subject.execute
end
it 'merges variables returned by all steps' do
expect(subject.execute).to eq(
status: :success,
variable1: 'var1',
variable2: 'var2'
)
end
end
context 'with multiple stepable classes' do
let(:other_class) do
Class.new do
include Stepable
steps :other_method1, :other_method2
private
def other_method1
{ status: :success }
end
def other_method2
{ status: :success }
end
end
end
it 'does not leak steps' do
expect(other_class.new.steps).to contain_exactly(:other_method1, :other_method2)
expect(subject.steps).to contain_exactly(:method1, :method2, :method3, :appended_method1)
end
end
end

View file

@ -0,0 +1,201 @@
# frozen_string_literal: true
require 'spec_helper'
describe SelfMonitoring::Project::CreateService do
describe '#execute' do
let(:result) { subject.execute }
let(:prometheus_settings) do
OpenStruct.new(
enable: true,
listen_address: 'localhost:9090'
)
end
before do
allow(Gitlab.config).to receive(:prometheus).and_return(prometheus_settings)
end
context 'without admin users' do
it 'returns error' do
expect(subject).to receive(:log_error).and_call_original
expect(result).to eq(
status: :error,
message: 'No active admin user found',
failed_step: :validate_admins
)
end
end
context 'with admin users' do
let(:project) { result[:project] }
let!(:user) { create(:user, :admin) }
before do
allow(ApplicationSetting)
.to receive(:current)
.and_return(
ApplicationSetting.build_from_defaults(allow_local_requests_from_hooks_and_services: true)
)
end
shared_examples 'has prometheus service' do |listen_address|
it do
expect(result[:status]).to eq(:success)
prometheus = project.prometheus_service
expect(prometheus).not_to eq(nil)
expect(prometheus.api_url).to eq(listen_address)
expect(prometheus.active).to eq(true)
expect(prometheus.manual_configuration).to eq(true)
end
end
it_behaves_like 'has prometheus service', 'http://localhost:9090'
it 'creates project with internal visibility' do
expect(result[:status]).to eq(:success)
expect(project.visibility_level).to eq(Gitlab::VisibilityLevel::INTERNAL)
expect(project).to be_persisted
end
it 'creates project with internal visibility even when internal visibility is restricted' do
stub_application_setting(restricted_visibility_levels: [Gitlab::VisibilityLevel::INTERNAL])
expect(result[:status]).to eq(:success)
expect(project.visibility_level).to eq(Gitlab::VisibilityLevel::INTERNAL)
expect(project).to be_persisted
end
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)
end
it 'adds all admins as maintainers' do
admin1 = create(:user, :admin)
admin2 = create(:user, :admin)
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,
Gitlab::Access::MAINTAINER,
Gitlab::Access::MAINTAINER
)
end
# This should pass when https://gitlab.com/gitlab-org/gitlab-ce/issues/44496
# is complete and the prometheus listen address is added to the whitelist.
# 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_hooks_and_services: false)
# )
# end
# it_behaves_like 'has prometheus service', 'http://localhost:9090'
# end
context 'with non default prometheus address' do
before do
prometheus_settings.listen_address = 'https://localhost:9090'
end
it_behaves_like 'has prometheus service', 'https://localhost:9090'
end
context 'when prometheus setting is not present in gitlab.yml' do
before do
allow(Gitlab.config).to receive(:prometheus).and_raise(Settingslogic::MissingSetting)
end
it 'does not fail' do
expect(result).to include(status: :success)
expect(project.prometheus_service).to be_nil
end
end
context 'when prometheus setting is disabled in gitlab.yml' do
before do
prometheus_settings.enable = false
end
it 'does not configure prometheus' do
expect(result).to include(status: :success)
expect(project.prometheus_service).to be_nil
end
end
context 'when prometheus listen address is blank in gitlab.yml' do
before do
prometheus_settings.listen_address = ''
end
it 'does not configure prometheus' do
expect(result).to include(status: :success)
expect(project.prometheus_service).to be_nil
end
end
context 'when project cannot be created' do
let(:project) { build(:project) }
before do
project.errors.add(:base, "Test error")
expect_next_instance_of(::Projects::CreateService) do |project_create_service|
expect(project_create_service).to receive(:execute)
.and_return(project)
end
end
it 'returns error' do
expect(subject).to receive(:log_error).and_call_original
expect(result).to eq({
status: :error,
message: 'Could not create project',
failed_step: :create_project
})
end
end
context 'when user cannot be added to project' do
before do
subject.instance_variable_set(:@instance_admins, [user, build(:user, :admin)])
end
it 'returns error' do
expect(subject).to receive(:log_error).and_call_original
expect(result).to eq({
status: :error,
message: 'Could not add admins as members',
failed_step: :add_project_members
})
end
end
context 'when prometheus manual configuration cannot be saved' do
before do
prometheus_settings.listen_address = 'httpinvalid://localhost:9090'
end
it 'returns error' do
expect(subject).to receive(:log_error).and_call_original
expect(result).to eq(
status: :error,
message: 'Could not save prometheus manual configuration',
failed_step: :add_prometheus_manual_configuration
)
end
end
end
end
end