Alternative proposal to prevent destructive actions on production databases

Issue: https://github.com/rails/rails/pull/21237

Pull-request: https://github.com/rails/rails/pull/22967

Hi!

This past January, Richard Schneeman came up with a idea for preventing undesired destructive actions on the production database.

Good!

It was implemented via the introduction of a key/value table to store internal Active Record values (namely “ar_internal_metadata”). This table stores the “environment” that was used when migrating the database. Before executing any tests, it will read this value, compare it agains the list of “protected_environments”, and abort the operation if it’s unsafe.

However, people expressed some concerns related to the database pollution this creates.

I would like to share a proposal:

Before doing any destructive action, Rails should check if the configuration of the connection that is being used is the same as the one in the list of “protected_environments” and prompt for confirmation: “it seems you are trying to execute db:drop task using the same configuration as the production database, do you want to continue? [Y/n]”.

This option should work in cases in which the production credentials are introduced in development for diagnosing an issue or using ENV[“DATABASE_URL”] for all the environments.

In addition, if we decide to go down this road, “protected_environments” could be marked in the “config/database.yml” configuration file.

However, people expressed some concerns related to the database pollution this creates.

I would like to share a proposal:

Thanks, but as you noted, this was discussed and decided 6 months ago, and has now shipped.

That doesn’t preclude all further discussion, of course.. but it does mean the bar is measurably higher: we’re not going to implement two of these mechanisms, so a proposal is now a suggestion that we deprecate and remove the existing functionality, and force people to migrate. Clearly, that effort and inconvenience would require a specific, compelling argument.

Matthew

Hi Matthew,

I completely understand.

I would like to give it another chance:

The migration process of the new proposal should be backwards compatible, the people could ignore the “ar_internal_metadata” table or delete it (even include a deprecation warning). If I assume the effort of the implementation, could that be something to be considered?

The ar_internal_metadata is tiny. I wouldn’t consider it database pollution any more than the schema migration table. It’s actually much smaller.

Rails should check if the configuration of the connection that is being used is the same as the one in the list of “protected_environments”

This doesn’t handle the case where you pull down your production configuration and put it in your development group because you wanted to debug something locally that required production data. People do this. It doesn’t handle the case where people are using DATABASE_URL but don’t have it specified in their config/database.yml.

If we switched from storing the data anywhere but in the database we run the risk of mistakes. I didn’t want a feature that required you be extra careful to be safe, I wanted a system where you’re safe® even when you’re not being careful.

I appreciate thinking about alternate implementation ideas. In this case optimizing to remove ar_internal_metadata table is not a very worthwhile endeavor in my opinion.

Understood. Thanks Richard!