Expose command line option to configure umask for directories and files

restic / rest-server

Rest Server is a high performance HTTP server that implements restic's REST backend API.

BSD 2-Clause "Simplified" License

978 stars 143 forks source link

Expose command line option to configure umask for directories and files #190

Open mlusetti opened 2 years ago

mlusetti commented 2 years ago

What is the purpose of this change? What does it change?

This will fix #189

Was the change discussed in an issue or in the forum before?

I proposed in #189

Checklist

[X] I have enabled maintainer edits for this PR
[ ] I have added tests for all changes in this PR
[X] I have added documentation for the changes (in the manual)
[X] There's a new file in changelog/unreleased/ that describes the changes for our users (template here)
[X] I have run gofmt on the code in all commits
[X] All commit messages are formatted in the same style as the other commits in the repo
[X] I'm done, this Pull Request is ready for review

mlusetti commented 2 years ago

The test/lint check fail on code I haven't touched, so I guess is failing the same way in the master branch too

fd0 commented 2 years ago

I've fixed this in master (sorry about that) and also raised the min Go version to 1.15. Unfortunately you haven't enabled maintainer edits, so I'm unable to push the fixed commits. Can you please enable that? Thanks!

mlusetti commented 2 years ago

I've fixed this in master (sorry about that) and also raised the min Go version to 1.15. Unfortunately you haven't enabled maintainer edits, so I'm unable to push the fixed commits. Can you please enable that? Thanks!

I've checked "Allow edits by maintainers" flag ... Should I do anything more ?

fd0 commented 2 years ago

Hm, no, that's exactly the setting. Maybe I did something wrong? I'll have a look

fd0 commented 2 years ago

Sigh, PEBKAC, I tried to push to your "restic" repo, instead of the "rest-server" repo. :grin:

mlusetti commented 2 years ago

In my env it didn't fail to build on Go 1.15 and io/fs is not present in the source code

mlusetti commented 2 years ago

I reworked your diff cause it has lost (a rebase?) my last commit

mlusetti commented 2 years ago

Hi @fd0 do you think there's need for other changes in the diff ?

MichaelEischer commented 2 years ago

@mlusetti I think fd0 intentionally removed the "make it go 1.14 buildable" commit as it is no longer necessary after rebasing to the current master branch.

mlusetti commented 2 years ago

@mlusetti I think fd0 intentionally removed the "make it go 1.14 buildable" commit as it is no longer necessary after rebasing to the current master branch.

Thanks for your pointer but as far as I can see io/fs package is not in GOROOT in 1.15 too ... Am I missing something ?

ianneub commented 2 years ago

Is there anything that I could help with to get this PR merged? I could use this patch for a situation very similar to @mlusetti . I am trying to backup the restic files from a user that is different than the rest-server's user. Being able to set new file permissions to 660 for new files would allow this to work in my situation.

Thanks so much for everyone's efforts thus far on this patch and Restic in general!

mlusetti commented 2 years ago

Any news on this one ?

To me is a simple diff which don't introduce backward incompatibilities nor change any default behaviors.

Thanks

rawtaz commented 2 years ago

I'm so stupid. I asked over two months ago in #189 what the more specific use case was, which @mlusetti kindly answered. @MichaelEischer also commented. But I looked for an answer here in this PR.

So, the use case is to be able to let groups read the repository files. This is pretty much in line with a similar change introduced in restic here: https://github.com/restic/restic/pull/3419/files - it allows for group access to the repository files.

Generally speaking our preference would be to have the same behavior here in rest-server to what we have there in restic, or at least restrict the options to the current default and group-readable. We feel the PR as it is now is too allowing basically, we'd like to meet the specific use case instead of opening up to potential mishaps, at least for now.

mlusetti commented 1 year ago

Tried to rebase commits since a lot has gone through master since the initial PR. If you prefer we could close this PR and I'll file another one with the current impl branched from current master

eloo commented 7 months ago

Hi there,

was the current state of this PR? I'm right running in the same issue as i want to backup my PVC using VolSync to the restic-rest-server but the file permissions making it tricky to access the backup afterwards from my local machine.

I guess this PR would fix this because i can now use my shared group permissions to access the files.

Thanks

MichaelEischer commented 7 months ago

@mlusetti Are you still interested in finishing this PR? Or does someone else want to take over?

mlusetti commented 6 months ago

@mlusetti Are you still interested in finishing this PR? Or does someone else want to take over?

Yep, currently I'm using a slightly modified version of the patch that suit my needs.

I can try to see if I've some spare cycle, no guarantees. If anyone faster that would be nice too.

MichaelEischer commented 6 months ago

I can try to see if I've some spare cycle, no guarantees. If anyone faster that would be nice too.

Just open a new PR once you're ready and refer to this one. Then I'll close the old one.