Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a persistent volume check. #2501

Merged

Conversation

BlackDex
Copy link
Collaborator

This will add a persistent volume check to make sure when running
containers someone is using a volume for persistent storage.

This check can be bypassed if someone configures
I_REALLY_WANT_VOLATILE_STORAGE=true as an environment variable.

This should prevent issues like #2493 .

Copy link
Contributor

@williamdes williamdes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not convinced reading the diff on how it does work, can you explain a bit more ?

@BlackDex
Copy link
Collaborator Author

I am not convinced reading the diff on how it does work, can you explain a bit more ?

Within the container it creates a special file. This special file is checked within the code. If this file exists then Vaultwarden will exit.

That file should disappear if docker mounts over the data folder, and hence that file should be gone.

@williamdes
Copy link
Contributor

I am not convinced reading the diff on how it does work, can you explain a bit more ?

Within the container it creates a special file. This special file is checked within the code. If this file exists then Vaultwarden will exit.

That file should disappear if docker mounts over the data folder, and hence that file should be gone.

Aha !
Maybe just add one more comment line that says something like "The /data/ volume will remove this file and this is proves persistence is setup"

@BlackDex BlackDex force-pushed the add-persistent-volume-check-docker branch from 7390053 to f944be0 Compare May 25, 2022 15:35
@BlackDex
Copy link
Collaborator Author

I am not convinced reading the diff on how it does work, can you explain a bit more ?

Within the container it creates a special file. This special file is checked within the code. If this file exists then Vaultwarden will exit.
That file should disappear if docker mounts over the data folder, and hence that file should be gone.

Aha ! Maybe just add one more comment line that says something like "The /data/ volume will remove this file and this is proves persistence is setup"

Done :)

Copy link
Contributor

@williamdes williamdes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect, missing Dockerfile updates from template but okay :)

@BlackDex
Copy link
Collaborator Author

Meh forgot to run make. Will do that later

src/main.rs Outdated Show resolved Hide resolved
This will add a persistent volume check to make sure when running
containers someone is using a volume for persistent storage.

This check can be bypassed if someone configures
`I_REALLY_WANT_VOLATILE_STORAGE=true` as an environment variable.

This should prevent issues like dani-garcia#2493 .
@BlackDex BlackDex force-pushed the add-persistent-volume-check-docker branch from f944be0 to 40ed505 Compare May 26, 2022 07:40
@dani-garcia dani-garcia merged commit 5845ed2 into dani-garcia:main May 27, 2022
@BlackDex BlackDex deleted the add-persistent-volume-check-docker branch May 27, 2022 18:29
BlackDex added a commit to BlackDex/vaultwarden that referenced this pull request May 28, 2022
It seemed there were some issues building the cross-platform images.
This PR fixes dani-garcia#2501 so building the containers will work again.
BlackDex added a commit to BlackDex/vaultwarden that referenced this pull request May 28, 2022
It seemed there were some issues building the cross-platform images.
This PR fixes dani-garcia#2501 so building the containers will work again.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants