diff --git a/config/initializers/6_validations.rb b/config/initializers/6_validations.rb index d92cdb97766..89aabe530fe 100644 --- a/config/initializers/6_validations.rb +++ b/config/initializers/6_validations.rb @@ -26,17 +26,6 @@ def validate_storages_config Gitlab.config.repositories.storages.each do |name, repository_storage| storage_validation_error("\"#{name}\" is not a valid storage name") unless storage_name_valid?(name) - if repository_storage.is_a?(String) - raise "#{name} is not a valid storage, because it has no `path` key. " \ - "It may be configured as:\n\n#{name}:\n path: #{repository_storage}\n\n" \ - "For source installations, update your config/gitlab.yml Refer to gitlab.yml.example for an updated example.\n\n" \ - "If you're using the Gitlab Development Kit, you can update your configuration running `gdk reconfigure`.\n" - end - - if !repository_storage.is_a?(Gitlab::GitalyClient::StorageSettings) || repository_storage.legacy_disk_path.nil? - storage_validation_error("#{name} is not a valid storage, because it has no `path` key. Refer to gitlab.yml.example for an updated example") - end - %w(failure_count_threshold failure_reset_time storage_timeout).each do |setting| # Falling back to the defaults is fine! next if repository_storage[setting].nil? diff --git a/lib/gitlab/gitaly_client/storage_settings.rb b/lib/gitlab/gitaly_client/storage_settings.rb index 8668caf0c55..9a576e463e3 100644 --- a/lib/gitlab/gitaly_client/storage_settings.rb +++ b/lib/gitlab/gitaly_client/storage_settings.rb @@ -5,6 +5,14 @@ module Gitlab # directly. class StorageSettings DirectPathAccessError = Class.new(StandardError) + InvalidConfigurationError = Class.new(StandardError) + + INVALID_STORAGE_MESSAGE = <<~MSG.freeze + Storage is invalid because it has no `path` key. + + For source installations, update your config/gitlab.yml Refer to gitlab.yml.example for an updated example. + If you're using the Gitlab Development Kit, you can update your configuration running `gdk reconfigure`. + MSG # This class will give easily recognizable NoMethodErrors Deprecated = Class.new @@ -12,7 +20,8 @@ module Gitlab attr_reader :legacy_disk_path def initialize(storage) - raise "expected a Hash, got a #{storage.class.name}" unless storage.is_a?(Hash) + raise InvalidConfigurationError, "expected a Hash, got a #{storage.class.name}" unless storage.is_a?(Hash) + raise InvalidConfigurationError, INVALID_STORAGE_MESSAGE unless storage.has_key?('path') # Support a nil 'path' field because some of the circuit breaker tests use it. @legacy_disk_path = File.expand_path(storage['path'], Rails.root) if storage['path'] diff --git a/spec/initializers/6_validations_spec.rb b/spec/initializers/6_validations_spec.rb index 1dc307ea922..8d9dc092547 100644 --- a/spec/initializers/6_validations_spec.rb +++ b/spec/initializers/6_validations_spec.rb @@ -42,26 +42,6 @@ describe '6_validations' do expect { validate_storages_config }.to raise_error('"name with spaces" is not a valid storage name. Please fix this in your gitlab.yml before starting GitLab.') end end - - context 'with incomplete settings' do - before do - mock_storages('foo' => {}) - end - - it 'throws an error suggesting the user to update its settings' do - expect { validate_storages_config }.to raise_error('foo is not a valid storage, because it has no `path` key. Refer to gitlab.yml.example for an updated example. Please fix this in your gitlab.yml before starting GitLab.') - end - end - - context 'with deprecated settings structure' do - before do - mock_storages('foo' => 'tmp/tests/paths/a/b/c') - end - - it 'throws an error suggesting the user to update its settings' do - expect { validate_storages_config }.to raise_error("foo is not a valid storage, because it has no `path` key. It may be configured as:\n\nfoo:\n path: tmp/tests/paths/a/b/c\n\nFor source installations, update your config/gitlab.yml Refer to gitlab.yml.example for an updated example.\n\nIf you're using the Gitlab Development Kit, you can update your configuration running `gdk reconfigure`.\n") - end - end end describe 'validate_storages_paths' do diff --git a/spec/lib/gitlab/gitaly_client/storage_settings_spec.rb b/spec/lib/gitlab/gitaly_client/storage_settings_spec.rb new file mode 100644 index 00000000000..c89913ec8e9 --- /dev/null +++ b/spec/lib/gitlab/gitaly_client/storage_settings_spec.rb @@ -0,0 +1,29 @@ +require 'spec_helper' + +describe Gitlab::GitalyClient::StorageSettings do + describe "#initialize" do + context 'when the storage contains no path' do + it 'raises an error' do + expect do + described_class.new("foo" => {}) + end.to raise_error(described_class::InvalidConfigurationError) + end + end + + context "when the argument isn't a hash" do + it 'raises an error' do + expect do + described_class.new("test") + end.to raise_error("expected a Hash, got a String") + end + end + + context 'when the storage is valid' do + it 'raises no error' do + expect do + described_class.new("path" => Rails.root) + end.not_to raise_error + end + end + end +end