From d3a9ead30d47512518dab7ab5293cc64987a0f04 Mon Sep 17 00:00:00 2001 From: Michael Baudino Date: Fri, 20 Mar 2026 17:27:09 +0100 Subject: [PATCH] Add `--backup-id` and `--backup-url` options to `development restore` command This commit adds 2 related options to `development restore`: * `--backup-id` allows to specify a backup id (as returned by the `heroku pg:backups` command) to restore, instead of automatically restoring the latest backup (which is still the default behavior) * `--backup-url` allows to specify the download URL of the backup to restore (as returned by the `heroku pg:backups:url` command), which can be useful to users without admin access to the Heroku application (and thus, who cannot execute `heroku pg:backups:url`): they can now fetch the backup from another channel (e.g. an admin team mate) and restore it locally as if it came directly from Heroku (with all the nice things that `development restore` provides, like reseting the local database, taking care of un-rememberable `pg_restore` arguments and updating environment in `ar_internal_metadata`). Incidentally, this commit also renames the temporary file used from `tmp/latest.backup` to `tmp/parity.backup`, since the backup stored temporarily is not always the latest one. --- README.md | 12 +++ lib/parity/backup.rb | 33 +++++++- lib/parity/environment.rb | 31 +++++++- spec/parity/backup_spec.rb | 116 ++++++++++++++++++++++++++- spec/parity/environment_spec.rb | 135 ++++++++++++++++++++++++++++++++ 5 files changed, 316 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 2a244bc..61d7e42 100644 --- a/README.md +++ b/README.md @@ -89,6 +89,18 @@ as a flag to the `development` or `production` commands: development restore-from production --parallelize ``` +When restoring to development, you can specify a specific backup using either: + +* `--backup-id` to download a specific backup by its Heroku backup ID: + ```shell + development restore production --backup-id a1234 + ``` + +* `--backup-url` to download from a specific URL (as returned by `heroku pg:backups:url`): + ```shell + development restore production --backup-url https://... + ``` + [2]: http://redis.io/commands Convention diff --git a/lib/parity/backup.rb b/lib/parity/backup.rb index 5fffea0..2d1bf8e 100644 --- a/lib/parity/backup.rb +++ b/lib/parity/backup.rb @@ -11,6 +11,8 @@ def initialize(args) @from, @to = args.values_at(:from, :to) @additional_args = args[:additional_args] || BLANK_ARGUMENTS @parallelize = args[:parallelize] || false + @backup_id = args[:backup_id] + @backup_url = args[:backup_url] end def restore @@ -25,7 +27,8 @@ def restore private - attr_reader :additional_args, :from, :to, :parallelize + attr_reader :additional_args, :backup_id, :backup_url, :from, :to, + :parallelize alias :parallelize? :parallelize @@ -77,21 +80,43 @@ def ensure_temp_directory_exists end def download_remote_backup + if backup_url + download_from_url + elsif backup_id + download_backup_by_id + else + download_latest_backup + end + end + + def download_from_url + Kernel.system("curl -o tmp/parity.backup \"#{backup_url}\"") + end + + def download_backup_by_id + Kernel.system( + "curl -o tmp/parity.backup "\ + "\"$(heroku pg:backups:url #{backup_id} --remote #{from})\"", + ) + end + + def download_latest_backup Kernel.system( - "curl -o tmp/latest.backup \"$(heroku pg:backups:url --remote #{from})\"", + "curl -o tmp/parity.backup "\ + "\"$(heroku pg:backups:url --remote #{from})\"", ) end def restore_from_local_temp_backup Kernel.system( - "pg_restore tmp/latest.backup --verbose --no-acl --no-owner "\ + "pg_restore tmp/parity.backup --verbose --no-acl --no-owner "\ "--dbname #{development_db} --jobs=#{processor_cores} "\ "#{additional_args}", ) end def delete_local_temp_backup - Kernel.system("rm tmp/latest.backup") + Kernel.system("rm tmp/parity.backup") end def delete_rails_production_environment_settings diff --git a/lib/parity/environment.rb b/lib/parity/environment.rb index f293ff4..4b9aad3 100644 --- a/lib/parity/environment.rb +++ b/lib/parity/environment.rb @@ -67,12 +67,14 @@ def restore $stdout.puts "Parity does not support restoring backups into your "\ "production environment. Use `--force` to override." else - Backup.new( + Backup.new(**{ from: arguments.first, to: environment, parallelize: parallelize?, + backup_id: backup_id, + backup_url: backup_url, additional_args: additional_restore_arguments, - ).restore + }.compact).restore end end @@ -90,9 +92,30 @@ def parallelize? arguments.include?("--parallelize") end + def backup_id + argument_value("--backup-id") + end + + def backup_url + argument_value("--backup-url") + end + + def argument_value(flag) + index = arguments.index(flag) + arguments[index + 1] if index + end + def additional_restore_arguments - (arguments.drop(1) - ["--force", "--parallelize"] + - [restore_confirmation_argument]).compact.join(" ") + args = arguments.dup + # Remove the remote environment name + args = args.drop(1) + # Remove toggle options --force and --parallelize + args -= ["--force", "--parallelize"] + # Remove key/value options + args -= ["--backup-id", backup_id] if backup_id + args -= ["--backup-url", backup_url] if backup_url + + (args + [restore_confirmation_argument]).compact.join(" ") end def restore_confirmation_argument diff --git a/spec/parity/backup_spec.rb b/spec/parity/backup_spec.rb index 2a5e55b..73d0d3e 100644 --- a/spec/parity/backup_spec.rb +++ b/spec/parity/backup_spec.rb @@ -147,6 +147,103 @@ expect(Kernel).to have_received(:system).with(set_db_metadata_sql) end + it "downloads backup from URL when backup_url is specified" do + allow(IO).to receive(:read).and_return(database_fixture) + allow(Kernel).to receive(:system) + allow(Etc).to receive(:nprocessors).and_return(number_of_processes) + + Parity::Backup.new( + from: "production", + to: "development", + backup_url: "https://example.com/backup.dump", + parallelize: true, + ).restore + + expect(Kernel). + to have_received(:system). + with(make_temp_directory_command) + expect(Kernel). + to have_received(:system). + with(download_from_url_command("https://example.com/backup.dump")) + expect(Kernel). + to have_received(:system). + with(drop_development_database_drop_command) + expect(Kernel). + to have_received(:system). + with(create_heroku_ext_schema_command) + expect(Kernel). + to have_received(:system). + with(restore_from_local_temp_backup_command) + expect(Kernel). + to have_received(:system). + with(delete_local_temp_backup_command) + end + + it "downloads specific backup by id when backup_id is specified" do + allow(IO).to receive(:read).and_return(database_fixture) + allow(Kernel).to receive(:system) + allow(Etc).to receive(:nprocessors).and_return(number_of_processes) + + Parity::Backup.new( + from: "production", + to: "development", + backup_id: "a1234", + parallelize: true, + ).restore + + expect(Kernel). + to have_received(:system). + with(make_temp_directory_command) + expect(Kernel). + to have_received(:system). + with(download_backup_by_id_command(id: "a1234", remote: "production")) + expect(Kernel). + to have_received(:system). + with(drop_development_database_drop_command) + expect(Kernel). + to have_received(:system). + with(create_heroku_ext_schema_command) + expect(Kernel). + to have_received(:system). + with(restore_from_local_temp_backup_command) + expect(Kernel). + to have_received(:system). + with(delete_local_temp_backup_command) + end + + it "prefers backup_url over backup_id when both are specified" do + allow(IO).to receive(:read).and_return(database_fixture) + allow(Kernel).to receive(:system) + allow(Etc).to receive(:nprocessors).and_return(number_of_processes) + + Parity::Backup.new( + from: "production", + to: "development", + backup_url: "https://example.com/backup.dump", + backup_id: "a1234", + parallelize: true, + ).restore + + expect(Kernel). + to have_received(:system). + with(make_temp_directory_command) + expect(Kernel). + to have_received(:system). + with(download_from_url_command("https://example.com/backup.dump")) + expect(Kernel). + not_to have_received(:system). + with(download_backup_by_id_command(id: "a1234", remote: "production")) + expect(Kernel). + to have_received(:system). + with(drop_development_database_drop_command) + expect(Kernel). + to have_received(:system). + with(restore_from_local_temp_backup_command) + expect(Kernel). + to have_received(:system). + with(delete_local_temp_backup_command) + end + it "is able to load a database.yml file containing top-level ERB" do allow(IO).to receive(:read).and_return(database_fixture_with_erb) allow(Kernel).to receive(:system) @@ -254,12 +351,25 @@ def make_temp_directory_command "mkdir -p tmp" end + def copy_local_backup_command(source:) + "cp \"#{source}\" tmp/parity.backup" + end + def download_remote_database_command - 'curl -o tmp/latest.backup "$(heroku pg:backups:url --remote production)"' + 'curl -o tmp/parity.backup "$(heroku pg:backups:url --remote production)"' + end + + def download_from_url_command(url) + "curl -o tmp/parity.backup \"#{url}\"" + end + + def download_backup_by_id_command(id:, remote:) + "curl -o tmp/parity.backup "\ + "\"$(heroku pg:backups:url #{id} --remote #{remote})\"" end def restore_from_local_temp_backup_command(cores: number_of_processes) - "pg_restore tmp/latest.backup --verbose --no-acl --no-owner "\ + "pg_restore tmp/parity.backup --verbose --no-acl --no-owner "\ "--dbname #{default_db_name} --jobs=#{cores} " end @@ -268,7 +378,7 @@ def number_of_processes end def delete_local_temp_backup_command - "rm tmp/latest.backup" + "rm tmp/parity.backup" end def heroku_development_to_staging_passthrough(db_name: default_db_name) diff --git a/spec/parity/environment_spec.rb b/spec/parity/environment_spec.rb index b5df78d..b163043 100644 --- a/spec/parity/environment_spec.rb +++ b/spec/parity/environment_spec.rb @@ -194,6 +194,141 @@ expect(backup).to have_received(:restore) end + it "passes backup_id when --backup-id is specified" do + backup = stub_parity_backup + allow(Parity::Backup).to receive(:new).and_return(backup) + + Parity::Environment.new( + "development", + ["restore", "production", "--backup-id", "a1234"], + ).run + + expect(Parity::Backup).to have_received(:new). + with( + from: "production", + to: "development", + parallelize: false, + backup_id: "a1234", + additional_args: "", + ) + expect(backup).to have_received(:restore) + end + + it "passes backup_url when --backup-url is specified" do + backup = stub_parity_backup + allow(Parity::Backup).to receive(:new).and_return(backup) + + Parity::Environment.new( + "development", + [ + "restore", "production", + "--backup-url", "https://example.com/backup.dump" + ], + ).run + + expect(Parity::Backup).to have_received(:new). + with( + from: "production", + to: "development", + parallelize: false, + backup_url: "https://example.com/backup.dump", + additional_args: "", + ) + expect(backup).to have_received(:restore) + end + + it "passes backup_id with parallelize and strips flag from additional args" do + backup = stub_parity_backup + allow(Parity::Backup).to receive(:new).and_return(backup) + + Parity::Environment.new( + "development", + [ + "restore", "production", + "--backup-id", "b5678", + "--parallelize" + ], + ).run + + expect(Parity::Backup).to have_received(:new). + with( + from: "production", + to: "development", + parallelize: true, + backup_id: "b5678", + additional_args: "", + ) + expect(backup).to have_received(:restore) + end + + it "passes backup_url with parallelize & strips flag from additional args" do + backup = stub_parity_backup + allow(Parity::Backup).to receive(:new).and_return(backup) + + Parity::Environment.new( + "development", + [ + "restore", "production", + "--backup-url", "https://example.com/dump", + "--parallelize" + ], + ).run + + expect(Parity::Backup).to have_received(:new). + with( + from: "production", + to: "development", + parallelize: true, + backup_url: "https://example.com/dump", + additional_args: "", + ) + expect(backup).to have_received(:restore) + end + + it "does not include --backup-id or its value in additional_args" do + backup = stub_parity_backup + allow(Parity::Backup).to receive(:new).and_return(backup) + + Parity::Environment.new( + "development", + ["restore", "production", "--backup-id", "a1234", "--verbose"], + ).run + + expect(Parity::Backup).to have_received(:new). + with( + from: "production", + to: "development", + parallelize: false, + backup_id: "a1234", + additional_args: "--verbose", + ) + expect(backup).to have_received(:restore) + end + + it "does not include --backup-url or its value in additional_args" do + backup = stub_parity_backup + allow(Parity::Backup).to receive(:new).and_return(backup) + + Parity::Environment.new( + "development", + [ + "restore", "production", + "--backup-url", "https://example.com/backup.dump", + "--verbose" + ], + ).run + + expect(Parity::Backup).to have_received(:new). + with( + from: "production", + to: "development", + parallelize: false, + backup_url: "https://example.com/backup.dump", + additional_args: "--verbose", + ) + expect(backup).to have_received(:restore) + end + it "opens the remote console" do Parity::Environment.new("production", ["console"]).run