heroku / legacy-cli

Heroku CLI
https://cli.heroku.com
MIT License
1.37k stars 380 forks source link

pgbackups:transfer allows meaningless same-database transfers #1181

Open fdr opened 10 years ago

fdr commented 10 years ago

Reported by a user who happened to not be using DATABASE_URL in the normal way -- it was pointing to a new, blank database -- and on account of positive feedback from toolbelt about the transfer wound up deprovisioning the database with all the data (while following the upgrade instructions).

This was because as the instructions indicated (and assuming a regular full-of-data DATABASE_URL) running heroku pgbackups:transfer NEWDATABASE would put data from DATABASE_URL into NEWDATABASE. In this instance the two were the same.

Demonstration setup:

$ heroku config -a sushi
=== sushi Config Vars
DATABASE_URL:                sameurl
HEROKU_POSTGRESQL_BLACK_URL:  sameurl

The following completes without protest to self-clean and restore. This is probably dangerous even without a misstep of how the defaults are intended to be used account of the "clean" option in pg_dump.

$ heroku pgbackups:transfer BLACK  -a sushi

 !    WARNING: Destructive Action
 !    Transfering data from DATABASE_URL to BLACK
 !    To proceed, type "sushi" or re-run this command with --confirm sushi

> sushi

Pg_dump... 2.25kB /
(completes successfully)

@heroku/department-of-data

msakrejda commented 10 years ago

Ha ha ha ha ha... :disappointed:

I think I can block this on the client easily enough. We may want to block it on the server as well, but I don't think we can relay a nice error message very easily given the current inerface.

fdr commented 10 years ago

I've since changed the instructions to hopefully call out the magical default DATABASE_URL interaction, but yeah, there are probably zero situations where this is good, by default or fat finger.

Null opinion on where the blocking should happen, as I'm not convinced I am in a position to handle it right now.

pvh commented 10 years ago

Maciek's both ends call seems reasonable. Measure twice, cut once.

On Thu, Aug 14, 2014 at 4:20 PM, Daniel Farina notifications@github.com wrote:

I've since changed the instructions to hopefully call out the magical default DATABASE_URL interaction, but yeah, there are probably zero situations where this is good, by default or fat finger.

Null opinion on where the blocking should happen, as I'm not convinced I am in a position to handle it right now.

— Reply to this email directly or view it on GitHub https://github.com/heroku/heroku/issues/1181#issuecomment-52257670.