-
-
Notifications
You must be signed in to change notification settings - Fork 86
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
feat: allow backups to be encrypted with age #432
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution, I am happy to add the feature and merge this. I left two questions for my understanding inline. In addition to that it'd be great if you could also add a test case for the new feature. You can probably just copy the existing gpg one and work off of that. If you need to install additional tools for the test container, here's the place to do it:
docker-volume-backup/test/Dockerfile
Lines 3 to 11 in bb11ae0
RUN apk add \ | |
coreutils \ | |
curl \ | |
gpg \ | |
jq \ | |
moreutils \ | |
tar \ | |
zstd \ | |
--no-cache |
18b7fb5
to
58daebf
Compare
@nkcmr thanks for the changes. Just to avoid misunderstandings: are you still planning on adding a test for the feature? No intent to pressure you into anything, just want to make sure you're not awaiting some sort of action from my end right now. |
@m90 Yes, sorry for not being more verbose. My morning only had enough time to do those edits before |
@nkcmr Sorry for being intrusive, but are you still interested in getting this merged? No worries if your mind wandered elsewhere, I'd just be interested to know if this shall be included in some release in the near future or not. Thanks. |
@m90 Yah sorry for being a flake about this. I will try to get this back on track in the next few days. My laptop broke right after I left my last comment, got it fixed now though! |
There's nothing I'd like to release right now, so take your time. Very happy to release this once it's done though, so let me know if I cam help with anything. |
2a36149
to
304ae35
Compare
At long last! I have rebased my original change and added tests. Sorry for taking so long! |
304ae35
to
4293c85
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks really great, thanks for all the effort you put into consolidating the encryption logic. I left two minor comments inline, nothing blocking. Let me know what you think, and we'll get this released soon.
6c3cc1b
to
2cabbdb
Compare
GPG is known to have usability issues and is generally cumbersome to use. age [0] is a modern alternative to GPG that is designed by a cryptographer that has worked and continues to work on Golang's crypto packages for years. Allowing age to be used to encrypt backups dramatically simplifies the backup process. [0]: https://age-encryption.org/
2cabbdb
to
5c3e87e
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great, thanks a lot 🎩 . I'll merge and release it once the build is green.
This is now included in v2.43.0 |
GPG is known to have usability issues and is generally cumbersome to use. age 0 is a modern alternative to GPG that is designed and implemented by a cryptographer that has worked and continues to work on Golang's crypto packages for years.
Allowing age to be used to encrypt backups dramatically simplifies the backup process.