From 0b18c956106719b6b79547f0135cbad4d91b1881 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Sun, 6 Sep 2020 22:33:18 -0700 Subject: [PATCH 1/2] Dynamically load Azure dependency for backups The `fog/azurerm` gem is not loaded by default in `Gemfile`. To make backups work, we need to load this dynamically when needed. --- lib/backup/manager.rb | 12 +++++++++++- spec/lib/backup/manager_spec.rb | 23 +++++++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/lib/backup/manager.rb b/lib/backup/manager.rb index 6bce9a2760df99..fe18c97ba9dd19 100644 --- a/lib/backup/manager.rb +++ b/lib/backup/manager.rb @@ -196,8 +196,10 @@ def available_timestamps end def connect_to_remote_directory(connection_settings) + settings = connection_settings.symbolize_keys + load_provider(settings) # our settings use string keys, but Fog expects symbols - connection = ::Fog::Storage.new(connection_settings.symbolize_keys) + connection = ::Fog::Storage.new(settings) # We only attempt to create the directory for local backups. For AWS # and other cloud providers, we cannot guarantee the user will have @@ -209,6 +211,14 @@ def connect_to_remote_directory(connection_settings) end end + def load_provider(settings) + provider = settings[:provider] + + return unless provider.present? + + require 'fog/azurerm' if provider == 'AzureRM' + end + def remote_directory Gitlab.config.backup.upload.remote_directory end diff --git a/spec/lib/backup/manager_spec.rb b/spec/lib/backup/manager_spec.rb index 38a5c30506b175..feaca6164eb358 100644 --- a/spec/lib/backup/manager_spec.rb +++ b/spec/lib/backup/manager_spec.rb @@ -416,5 +416,28 @@ subject.upload end end + + context 'with AzureRM provider' do + before do + stub_backup_setting( + upload: { + connection: { + provider: 'AzureRM', + azure_storage_account_name: 'test-access-id', + azure_storage_access_key: 'secret' + }, + remote_directory: 'directory', + multipart_chunk_size: nil, + encryption: nil, + encryption_key: nil, + storage_class: nil + } + ) + end + + it 'loads the provider' do + expect { subject.upload }.not_to raise_error + end + end end end -- GitLab From da81bdb0f6b285fa69a94fa6ba9473fb072958a9 Mon Sep 17 00:00:00 2001 From: Stan Hu Date: Mon, 7 Sep 2020 09:40:16 -0700 Subject: [PATCH 2/2] Refactor Backup::Manager to use ObjectStorage::Config Since AzureRM is a constant in many places, centralize this with the other providers. --- config/initializers/direct_upload_support.rb | 6 ++- lib/backup/manager.rb | 20 +++------- lib/object_storage/config.rb | 20 ++++++++-- spec/lib/object_storage/config_spec.rb | 41 ++++++++++++++++++++ 4 files changed, 68 insertions(+), 19 deletions(-) diff --git a/config/initializers/direct_upload_support.rb b/config/initializers/direct_upload_support.rb index 94e90727f0cefb..919b80b79c0b7d 100644 --- a/config/initializers/direct_upload_support.rb +++ b/config/initializers/direct_upload_support.rb @@ -1,5 +1,7 @@ class DirectUploadsValidator - SUPPORTED_DIRECT_UPLOAD_PROVIDERS = %w(Google AWS AzureRM).freeze + SUPPORTED_DIRECT_UPLOAD_PROVIDERS = [ObjectStorage::Config::GOOGLE_PROVIDER, + ObjectStorage::Config::AWS_PROVIDER, + ObjectStorage::Config::AZURE_PROVIDER].freeze ValidationError = Class.new(StandardError) @@ -24,7 +26,7 @@ def verify!(uploader_type, object_store) def provider_loaded?(provider) return false unless SUPPORTED_DIRECT_UPLOAD_PROVIDERS.include?(provider) - require 'fog/azurerm' if provider == 'AzureRM' + require 'fog/azurerm' if provider == ObjectStorage::Config::AZURE_PROVIDER true end diff --git a/lib/backup/manager.rb b/lib/backup/manager.rb index fe18c97ba9dd19..2b28b30fd74b1c 100644 --- a/lib/backup/manager.rb +++ b/lib/backup/manager.rb @@ -47,7 +47,7 @@ def upload return end - directory = connect_to_remote_directory(connection_settings) + directory = connect_to_remote_directory(Gitlab.config.backup.upload) if directory.files.create(create_attributes) progress.puts "done".color(:green) @@ -195,11 +195,11 @@ def available_timestamps @backup_file_list.map {|item| item.gsub("#{FILE_NAME_SUFFIX}", "")} end - def connect_to_remote_directory(connection_settings) - settings = connection_settings.symbolize_keys - load_provider(settings) - # our settings use string keys, but Fog expects symbols - connection = ::Fog::Storage.new(settings) + def connect_to_remote_directory(options) + config = ObjectStorage::Config.new(options) + config.load_provider + + connection = ::Fog::Storage.new(config.credentials) # We only attempt to create the directory for local backups. For AWS # and other cloud providers, we cannot guarantee the user will have @@ -211,14 +211,6 @@ def connect_to_remote_directory(connection_settings) end end - def load_provider(settings) - provider = settings[:provider] - - return unless provider.present? - - require 'fog/azurerm' if provider == 'AzureRM' - end - def remote_directory Gitlab.config.backup.upload.remote_directory end diff --git a/lib/object_storage/config.rb b/lib/object_storage/config.rb index e91fda298802ef..cc536ce9b46ce3 100644 --- a/lib/object_storage/config.rb +++ b/lib/object_storage/config.rb @@ -2,12 +2,26 @@ module ObjectStorage class Config + AWS_PROVIDER = 'AWS' + AZURE_PROVIDER = 'AzureRM' + GOOGLE_PROVIDER = 'Google' + attr_reader :options def initialize(options) @options = options.to_hash.deep_symbolize_keys end + def load_provider + if aws? + require 'fog/aws' + elsif google? + require 'fog/google' + elsif azure? + require 'fog/azurerm' + end + end + def credentials @credentials ||= options[:connection] || {} end @@ -30,7 +44,7 @@ def consolidated_settings? # AWS-specific options def aws? - provider == 'AWS' + provider == AWS_PROVIDER end def use_iam_profile? @@ -61,11 +75,11 @@ def azure_storage_domain # End Azure-specific options def google? - provider == 'Google' + provider == GOOGLE_PROVIDER end def azure? - provider == 'AzureRM' + provider == AZURE_PROVIDER end def fog_attributes diff --git a/spec/lib/object_storage/config_spec.rb b/spec/lib/object_storage/config_spec.rb index a48b5100065f51..0ead2a1d2699e7 100644 --- a/spec/lib/object_storage/config_spec.rb +++ b/spec/lib/object_storage/config_spec.rb @@ -2,6 +2,7 @@ require 'fast_spec_helper' require 'rspec-parameterized' +require 'fog/core' RSpec.describe ObjectStorage::Config do using RSpec::Parameterized::TableSyntax @@ -35,6 +36,46 @@ subject { described_class.new(raw_config.as_json) } + describe '#load_provider' do + before do + subject.load_provider + end + + context 'with AWS' do + it 'registers AWS as a provider' do + expect(Fog.providers.keys).to include(:aws) + end + end + + context 'with Google' do + let(:credentials) do + { + provider: 'Google', + google_storage_access_key_id: 'GOOGLE_ACCESS_KEY_ID', + google_storage_secret_access_key: 'GOOGLE_SECRET_ACCESS_KEY' + } + end + + it 'registers Google as a provider' do + expect(Fog.providers.keys).to include(:google) + end + end + + context 'with Azure' do + let(:credentials) do + { + provider: 'AzureRM', + azure_storage_account_name: 'azuretest', + azure_storage_access_key: 'ABCD1234' + } + end + + it 'registers AzureRM as a provider' do + expect(Fog.providers.keys).to include(:azurerm) + end + end + end + describe '#credentials' do it { expect(subject.credentials).to eq(credentials) } end -- GitLab