rest-server icon indicating copy to clipboard operation
rest-server copied to clipboard

Expose command line option to configure umask for directories and files

Open mlusetti opened this issue 3 years ago • 18 comments
trafficstars

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 avatar Apr 14 '22 17:04 mlusetti

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

mlusetti avatar Apr 15 '22 07:04 mlusetti

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!

fd0 avatar Apr 15 '22 07:04 fd0

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 ?

mlusetti avatar Apr 15 '22 07:04 mlusetti

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

fd0 avatar Apr 15 '22 08:04 fd0

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

fd0 avatar Apr 15 '22 08:04 fd0

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

mlusetti avatar Apr 15 '22 09:04 mlusetti

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

mlusetti avatar Apr 15 '22 09:04 mlusetti

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

mlusetti avatar Apr 19 '22 07:04 mlusetti

@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.

MichaelEischer avatar Apr 20 '22 20:04 MichaelEischer

@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 ?

mlusetti avatar Apr 21 '22 06:04 mlusetti

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!

ianneub avatar Oct 10 '22 14:10 ianneub

Any news on this one ?

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

Thanks

mlusetti avatar Oct 25 '22 16:10 mlusetti

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.

rawtaz avatar Oct 25 '22 19:10 rawtaz

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

mlusetti avatar Sep 07 '23 14:09 mlusetti

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

eloo avatar Mar 29 '24 09:03 eloo

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

MichaelEischer avatar Mar 29 '24 16:03 MichaelEischer

@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.

mlusetti avatar Apr 03 '24 07:04 mlusetti

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.

MichaelEischer avatar Apr 04 '24 17:04 MichaelEischer