Merge pull request #18167 from NickLaMuro/magic-number-support-for-da…

…tabase-restores

Add support for magic check in EvmDatabaseOps

(cherry picked from commit 3b053f0)

lib/evm_database_ops.rb

@@ -89,12 +89,12 @@ def self.restore(db_opts, connect_opts = {})
     #   :username => 'samba_one',
     #   :password => 'Zug-drep5s',
 
-    uri = with_file_storage(:restore, db_opts, connect_opts) do |database_opts|
-      prepare_for_restore(database_opts[:local_file])
+    uri = with_file_storage(:restore, db_opts, connect_opts) do |database_opts, backup_type|
+      prepare_for_restore(database_opts[:local_file], backup_type)
 
       # remove all the connections before we restore; AR will reconnect on the next query
       ActiveRecord::Base.connection_pool.disconnect!
-      PostgresAdmin.restore(database_opts)
+      PostgresAdmin.restore(database_opts.merge(:backup_type => backup_type))
     end
     _log.info("[#{merged_db_opts(db_opts)[:dbname]}] database has been restored from file: [#{uri}]")
     uri
@@ -139,12 +139,16 @@ def self.restore(db_opts, connect_opts = {})
     MiqFileStorage.with_interface_class(connect_opts) do |file_storage|
       send_args = [uri, db_opts[:byte_count]].compact
 
-      # Okay, this probably seems a bit silly seeing that we just went to the
-      # trouble of doing a `.compact)`.  The intent is that with a restore, we
-      # are doing a `MiqFileStorage.download`, and the interface for that
-      # method is to pass a `nil` for the block form since we are streaming the
-      # data from the command that we are writting as part of the block.
-      send_args.unshift(nil) if action == :restore
+      if action == :restore
+        # `MiqFileStorage#download` requires a `nil` passed to the block form
+        # to accommodate streaming
+        send_args.unshift(nil)
+        magic_numbers = {
+          :pgdump     => PostgresAdmin::PG_DUMP_MAGIC,
+          :basebackup => PostgresAdmin::BASE_BACKUP_MAGIC
+        }
+        backup_type = file_storage.magic_number_for(uri, :accepted => magic_numbers)
+      end
 
       # Note:  `input_path` will always be a fifo stream (input coming from
       # PostgresAdmin, and the output going to the `uri`), since we want to
@@ -161,15 +165,19 @@ def self.restore(db_opts, connect_opts = {})
       # set `db_opts` local file to that stream.
       file_storage.send(STORAGE_ACTIONS_TO_METHODS[action], *send_args) do |input_path|
         db_opts[:local_file] = input_path
-        yield(db_opts)
+        if action == :restore
+          yield(db_opts, backup_type)
+        else
+          yield(db_opts)
+        end
       end
     end
 
     uri
   end
 
-  private_class_method def self.prepare_for_restore(filename)
-    backup_type = validate_backup_file_type(filename)
+  private_class_method def self.prepare_for_restore(filename, backup_type = nil)
+    backup_type ||= validate_backup_file_type(filename)
 
     if application_connections?
       message = "Database restore failed. Shut down all evmserverd processes before attempting a database restore"

spec/lib/evm_database_ops_spec.rb

@@ -152,11 +152,10 @@
       allow(MiqSmbSession).to receive(:runcmd)
       allow(MiqSmbSession).to receive(:raw_disconnect)
       allow(file_storage).to  receive(:settings_mount_point).and_return(tmpdir)
+      allow(file_storage).to  receive(:magic_number_for).and_return(:pgdump)
       allow(file_storage).to  receive(:download).and_yield(input_path)
 
       allow(PostgresAdmin).to receive(:runcmd_with_logging)
-      allow(PostgresAdmin).to receive(:pg_dump_file?).and_return(true)
-      allow(PostgresAdmin).to receive(:base_backup_file?).and_return(false)
 
       allow(VmdbDatabaseConnection).to receive(:count).and_return(1)
     end
@@ -283,9 +282,18 @@ def execute_with_file_storage(action = :backup)
         it "returns calculated uri" do
           expect(execute_with_file_storage { "block result" }).to eq("smb://tmp/foo/db_backup/baz")
         end
+
+        it "yields `db_opt`s only" do
+          allow(file_storage).to receive(:download) { |&block| block.call(input_path) }
+          expect do |rspec_probe|
+            described_class.send(:with_file_storage, :backup, db_opts, connect_opts, &rspec_probe)
+          end.to yield_with_args(:dbname => "vmdb_production", :local_file => input_path)
+        end
       end
 
       context "for a restore action" do
+        before { expect(file_storage).to receive(:magic_number_for).and_return(:pgdump) }
+
         it "updates db_opts[:local_file] in the method context" do
           expect(file_storage).to receive(:send).with(:download, nil, "smb://tmp/foo")
           execute_with_file_storage(:restore)
@@ -303,6 +311,17 @@ def execute_with_file_storage(action = :backup)
           expect(execute_with_file_storage(:restore) { "block result" }).to eq("smb://tmp/foo")
         end
 
+        it "yields `backup_type` along with `db_opt`s" do
+          allow(file_storage).to receive(:download) { |&block| block.call(input_path) }
+          expected_yield_args = [
+            { :dbname => "vmdb_production", :local_file => input_path },
+            :pgdump
+          ]
+          expect do |rspec_probe|
+            described_class.send(:with_file_storage, :restore, db_opts, connect_opts, &rspec_probe)
+          end.to yield_with_args(*expected_yield_args)
+        end
+
         context "with query_params in the URI" do
           let(:connect_opts) { { :uri => "swift://container/foo.gz?2plus2=5" } }