hashicorp / vault

A tool for secrets management, encryption as a service, and privileged access management
https://www.vaultproject.io/
Other
30.94k stars 4.18k forks source link

Ability to stop writes to the filesystem backend #7191

Closed devicenull closed 6 months ago

devicenull commented 5 years ago

Is your feature request related to a problem? Please describe. Per this mailing list thread, vault's filesystem based database is not atomic, and may not always be in a state where it can be backed up. The only current option would be to halt/seal the vault, copy a backup, and restart/unseal. This is pretty disruptive, and does not lend itself well to automation.

Describe the solution you'd like I'd like to see a vault operator flush command, which would ensure that the database has been written to disk and is in a consistent state. This would ensure backups can be done without worrying about if the database is corrupt.

Alternatively, a vault operator backup command which would write out the database to a given path would work.

See also #5683, where there's no documentation about how we should actually be backing up this data.

siepkes commented 5 years ago

As my 2 cents I think a vault operator backup command which writes a consistent dump of the encrypted data to stdout (kind of like pg_dump for PostgreSQL) would be preferable (instead of a file). Makes it easier to integrate with various backup tools.

bbros-dev commented 4 years ago

In the interest of conserving scarce developer cycles....

Can this be closed in the absence of a specific API call failure being given?

The only current option would be to halt/seal the vault, copy a backup, and restart/unseal. This is pretty disruptive, and does not lend itself well to automation.

This is perfectly automated with the API. The documentation already contain examples of API calls (using curl) to initialize Vault, unseal, etc, etc. Whether this is disruptive or not depends on how the user has architectured their application - My 2c: Vault should steer clear of imposing constraints on how we write our applications.

Whether the operating system/file system (or any other backend) the user chose suppports 'atomic' actions is really outside of Vault's domain. Again my 2c: Vault should not require backends that support 'atomic' (however defined) actions. Similarly, Vault should not suggest that 'atomic' actions are best practice - please stick with the current behavior - Vault returns an error or not. Trying to validate my applications state, or worse whether the backend supports someone's idea of atomicity is a time sink for scarce developer time.

File system snapshots and rollbacks are best left in the realm of the file systems; btrfs, zfs etc etc.

Vault either returns are error when something is wrong or not.

In the absence of a reproducible example of Vault reporting no error when there is an error, or vice versa, please consider closing.

P.S The Post Restoration Notes in the Consul backend backup documentation address several potential gotchas, but again the OP doesn't indicate what API functionality is missing or broken.

siepkes commented 4 years ago

This is perfectly automated with the API. The documentation already contain examples of API calls (using curl) to initialize Vault, unseal, etc, etc.

All moving parts are reliability risks. Automated shutdown, startup, unsealing, possible leader election that follows, etc. introduces moving parts and creates a risk to the availability of a Vault cluster.

Whether the operating system/file system (or any other backend) the user chose suppports 'atomic' actions is really outside of Vault's domain. Again my 2c: Vault should not require backends that support 'atomic' (however defined) actions.

You seem to be under the impression that a backend would require atomic transactions in order for Vault to support an API which provides a consistent dump. A backend does not have to support atomic operations in order for Vault to support a consistent snapshot. Vault could for example use a WAL like construct to ensure the running dump remains consistent.

PS.

Discussion and different viewpoints are fine but I think it is rather harmful that you try to shutdown this discussion ("Can this be closed") under the guise of "In the interest of conserving scarce developer cycles" to push your own viewpoints. Your doing the exact same thing in #5683 and I don't think it's constructive.

bbros-dev commented 4 years ago

Discussion and different viewpoints are fine but I think it is rather harmful that you try to shutdown this discussion ("Can this be closed") under the guise of "In the interest of conserving scarce developer cycles" to push your own viewpoints. Your doing the exact same thing in #5683 and I don't think it's constructive.

Apologies for coming across that way. I'm actually trying to prevent the opposite, maybe I should have rephrased, but I didn't want to appear to criticize:
Making Vault have a consistent state will impose a constraint on apps that use Vault.
I'm trying to avoid that.

I'm not trying to stop people who want to configure Vault, its backend and their app so that a consistency constraint is binding.

Back to the topic:

Automated shutdown, startup, unsealing, possible leader election that follows, etc. introduces moving parts and creates a risk to the availability of a Vault cluster.

Agreed.
Having Vault implement a convenience command that provides the shutdown-export-startup-unseal functionality does allow users to mitigate that risk - we get a single implementation with tests and some assurance it works in the wild.

However, requesting a convenience command to ease the need to write shutdown-export-startup-unseal API or CLI scripts is very different from asking that Vault should satisfy a consistent state constraint, or even the ability to stop writes to the filesystem backend - to do this properly in Vault could very easily become a nightmare - the devs would be re-implementing what is already available in btrfs etc. and that is non-trivial functionality.

Rather than trying to push my own viewpoints, I'm actually trying to stop a 'consistent state' requirement being introduced through a backdoor - albeit inadvertently.

Chris Hoffman's response in the group thread refer to in issue #5683 was very clear, precise and was no basis for all the alarm-ism in that thread.

Backups are always based on the storage backend you are using and do require Vault to be offline to ensure consistency.

I didn't appreciate the FUD being peddled in that issue about Vault not being ready to be used in production settings.
It is difficult enough getting open source code used in many organizations without having to rebut those sorts of false claims/assertions, apologies for letting that irritation show.

Yes this issue should be closed.

The generic way to stop writes to the filesystem backend is exactly as Chris explained - shutdown Vault. Alternatively, if your OS/filesystem (and your application) supports it you might like to take a snapshot as @jefferai helpfully suggested as an alternative.

SayakMukhopadhyay commented 4 years ago

@bbros-dev Whats your angle here? Why are you trying so hard to stop discussing this topic? You have written a lot trying to trump all arguments made by others, both here and in #5683. What you must understand is that this is an open forum and users of open source software are free to ask feature enhancement or report something that they think is a bug. And seeing the sheer number of users looking for a change is what matters more than your attempts against them.

I would rather have the Vault team label this as a won't fix than having someone from outside the org and calling out to close this issue.

Seriously, why exactly do you want to close these issues?

bbros-dev commented 4 years ago

@bbros-dev Whats your angle here?

Rather than trying to push my own viewpoints, I'm actually trying to stop a 'consistent state' requirement being introduced through a backdoor - albeit inadvertently.

If you want the "Ability to stop writes to the filesystem backend" you already have that ability - vault seal. If you find the seal unseal dance is inconvenient you have the Vault Auto-Unseal functionality.

Finally, if you're not going to just relabel the current seal function there is the near certainty that vault will start doing more fsync(), etc. and that can hammer performance - and that is not just me saying that:

Data-to-disk synchronization calls are typically very slow on modern file systems and their extensive usage might slow down software significantly.

In short: High risk of degraded performance for no new functionality.

heatherezell commented 6 months ago

It appears this discussion veered off-topic. Can we verify whether this is still needed (aka, has not yet been addressed in newer versions of vault) and what exactly the ask is? Otherwise, let me know if it's okay to close. Thanks!

devicenull commented 6 months ago

I no longer work for the same company as I was when I requested this feature, so I guess you can call it resolved?

heatherezell commented 6 months ago

Closing as per recent comment