Merge branch 'feature/backup-custom-path' into 'master'

Support custom directory in gitlab:backup:create task

See merge request !12984

changelogs/unreleased/feature-backup-custom-path.yml

+---
+title: Support custom directory in gitlab:backup:create task
+merge_request: 12984
+author: Markus Koller

config/initializers/1_settings.rb

@@ -459,10 +459,6 @@ Settings.backup['pg_schema']    = nil
 Settings.backup['path']         = Settings.absolute(Settings.backup['path'] || "tmp/backups/")
 Settings.backup['archive_permissions'] ||= 0600
 Settings.backup['upload'] ||= Settingslogic.new({ 'remote_directory' => nil, 'connection' => nil })
-# Convert upload connection settings to use symbol keys, to make Fog happy
-if Settings.backup['upload']['connection']
-  Settings.backup['upload']['connection'] = Hash[Settings.backup['upload']['connection'].map { |k, v| [k.to_sym, v] }]
-end
 Settings.backup['upload']['multipart_chunk_size'] ||= 104857600
 Settings.backup['upload']['encryption'] ||= nil
 Settings.backup['upload']['storage_class'] ||= nil

doc/raketasks/backup_restore.md

@@ -270,6 +270,15 @@ For installations from source:
       remote_directory: 'gitlab_backups'
 ```
 
+### Specifying a custom directory for backups
+
+If you want to group your backups you can pass a `DIRECTORY` environment variable:
+
+```
+sudo gitlab-rake gitlab:backup:create DIRECTORY=daily
+sudo gitlab-rake gitlab:backup:create DIRECTORY=weekly
+```
+
 ### Backup archive permissions
 
 The backup archives created by GitLab (`1393513186_2014_02_27_gitlab_backup.tar`)

lib/backup/manager.rb

@@ -8,18 +8,9 @@ module Backup
       # Make sure there is a connection
       ActiveRecord::Base.connection.reconnect!
 
-      # saving additional informations
-      s = {}
-      s[:db_version]         = "#{ActiveRecord::Migrator.current_version}"
-      s[:backup_created_at]  = Time.now
-      s[:gitlab_version]     = Gitlab::VERSION
-      s[:tar_version]        = tar_version
-      s[:skipped]            = ENV["SKIP"]
-      tar_file = "#{s[:backup_created_at].strftime('%s_%Y_%m_%d_')}#{s[:gitlab_version]}#{FILE_NAME_SUFFIX}"
-
       Dir.chdir(backup_path) do
         File.open("#{backup_path}/backup_information.yml", "w+") do |file|
-          file << s.to_yaml.gsub(/^---\n/, '')
+          file << backup_information.to_yaml.gsub(/^---\n/, '')
         end
 
         # create archive
@@ -33,11 +24,11 @@ module Backup
           abort 'Backup failed'
         end
 
-        upload(tar_file)
+        upload
       end
     end
 
-    def upload(tar_file)
+    def upload
       $progress.print "Uploading backup archive to remote storage #{remote_directory} ... "
 
       connection_settings = Gitlab.config.backup.upload.connection
@@ -48,7 +39,7 @@ module Backup
 
       directory = connect_to_remote_directory(connection_settings)
 
-      if directory.files.create(key: tar_file, body: File.open(tar_file), public: false,
+      if directory.files.create(key: remote_target, body: File.open(tar_file), public: false,
                                 multipart_chunk_size: Gitlab.config.backup.upload.multipart_chunk_size,
                                 encryption: Gitlab.config.backup.upload.encryption,
                                 storage_class: Gitlab.config.backup.upload.storage_class)
@@ -177,7 +168,8 @@ module Backup
     end
 
     def connect_to_remote_directory(connection_settings)
-      connection = ::Fog::Storage.new(connection_settings)
+      # our settings use string keys, but Fog expects symbols
+      connection = ::Fog::Storage.new(connection_settings.symbolize_keys)
 
       # We only attempt to create the directory for local backups. For AWS
       # and other cloud providers, we cannot guarantee the user will have
@@ -193,6 +185,14 @@ module Backup
       Gitlab.config.backup.upload.remote_directory
     end
 
+    def remote_target
+      if ENV['DIRECTORY']
+        File.join(ENV['DIRECTORY'], tar_file)
+      else
+        tar_file
+      end
+    end
+
     def backup_contents
       folders_to_backup + archives_to_backup + ["backup_information.yml"]
     end
@@ -214,5 +214,19 @@ module Backup
     def settings
       @settings ||= YAML.load_file("backup_information.yml")
     end
+
+    def tar_file
+      @tar_file ||= "#{backup_information[:backup_created_at].strftime('%s_%Y_%m_%d_')}#{backup_information[:gitlab_version]}#{FILE_NAME_SUFFIX}"
+    end
+
+    def backup_information
+      @backup_information ||= {
+        db_version: ActiveRecord::Migrator.current_version.to_s,
+        backup_created_at: Time.now,
+        gitlab_version: Gitlab::VERSION,
+        tar_version: tar_version,
+        skipped: ENV["SKIP"]
+      }
+    end
   end
 end

spec/lib/gitlab/backup/manager_spec.rb

@@ -214,4 +214,56 @@ describe Backup::Manager, lib: true do
       end
     end
   end
+
+  describe '#upload' do
+    let(:backup_file) { Tempfile.new('backup', Gitlab.config.backup.path) }
+    let(:backup_filename) { File.basename(backup_file.path) }
+
+    before do
+      allow(subject).to receive(:tar_file).and_return(backup_filename)
+
+      stub_backup_setting(
+        upload: {
+          connection: {
+            provider: 'AWS',
+            aws_access_key_id: 'id',
+            aws_secret_access_key: 'secret'
+          },
+          remote_directory: 'directory',
+          multipart_chunk_size: 104857600,
+          encryption: nil,
+          storage_class: nil
+        }
+      )
+
+      # the Fog mock only knows about directories we create explicitly
+      Fog.mock!
+      connection = ::Fog::Storage.new(Gitlab.config.backup.upload.connection.symbolize_keys)
+      connection.directories.create(key: Gitlab.config.backup.upload.remote_directory)
+    end
+
+    context 'target path' do
+      it 'uses the tar filename by default' do
+        expect_any_instance_of(Fog::Collection).to receive(:create)
+          .with(hash_including(key: backup_filename))
+          .and_return(true)
+
+        Dir.chdir(Gitlab.config.backup.path) do
+          subject.upload
+        end
+      end
+
+      it 'adds the DIRECTORY environment variable if present' do
+        stub_env('DIRECTORY', 'daily')
+
+        expect_any_instance_of(Fog::Collection).to receive(:create)
+          .with(hash_including(key: "daily/#{backup_filename}"))
+          .and_return(true)
+
+        Dir.chdir(Gitlab.config.backup.path) do
+          subject.upload
+        end
+      end
+    end
+  end
 end

spec/support/stub_configuration.rb

@@ -4,9 +4,9 @@ module StubConfiguration
 
     # Stubbing both of these because we're not yet consistent with how we access
     # current application settings
-    allow_any_instance_of(ApplicationSetting).to receive_messages(messages)
+    allow_any_instance_of(ApplicationSetting).to receive_messages(to_settings(messages))
     allow(Gitlab::CurrentSettings.current_application_settings)
-      .to receive_messages(messages)
+      .to receive_messages(to_settings(messages))
   end
 
   def stub_not_protect_default_branch
@@ -15,23 +15,27 @@ module StubConfiguration
   end
 
   def stub_config_setting(messages)
-    allow(Gitlab.config.gitlab).to receive_messages(messages)
+    allow(Gitlab.config.gitlab).to receive_messages(to_settings(messages))
   end
 
   def stub_gravatar_setting(messages)
-    allow(Gitlab.config.gravatar).to receive_messages(messages)
+    allow(Gitlab.config.gravatar).to receive_messages(to_settings(messages))
   end
 
   def stub_incoming_email_setting(messages)
-    allow(Gitlab.config.incoming_email).to receive_messages(messages)
+    allow(Gitlab.config.incoming_email).to receive_messages(to_settings(messages))
   end
 
   def stub_mattermost_setting(messages)
-    allow(Gitlab.config.mattermost).to receive_messages(messages)
+    allow(Gitlab.config.mattermost).to receive_messages(to_settings(messages))
   end
 
   def stub_omniauth_setting(messages)
-    allow(Gitlab.config.omniauth).to receive_messages(messages)
+    allow(Gitlab.config.omniauth).to receive_messages(to_settings(messages))
+  end
+
+  def stub_backup_setting(messages)
+    allow(Gitlab.config.backup).to receive_messages(to_settings(messages))
   end
 
   private
@@ -54,4 +58,15 @@ module StubConfiguration
       messages[predicate.to_sym] = messages[key.to_sym]
     end
   end
+
+  # Support nested hashes by converting all values into Settingslogic objects
+  def to_settings(hash)
+    hash.transform_values do |value|
+      if value.is_a? Hash
+        Settingslogic.new(value.deep_stringify_keys)
+      else
+        value
+      end
+    end
+  end
 end