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

mlusetti · 2022-04-14T17:50:49Z

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

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

mlusetti · 2022-04-15T07:24:26Z

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 · 2022-04-15T07:42:10Z

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 · 2022-04-15T07:59:11Z

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 · 2022-04-15T08:24:30Z

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

fd0 · 2022-04-15T08:28:12Z

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

mlusetti · 2022-04-15T09:44:10Z

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

mlusetti · 2022-04-15T09:52:50Z

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

mlusetti · 2022-04-19T07:31:03Z

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

MichaelEischer · 2022-04-20T20:12:46Z

@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 · 2022-04-21T06:56:12Z

@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 · 2022-10-10T14:12:43Z

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 · 2022-10-25T16:46:31Z

Any news on this one ?

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

Thanks

rawtaz · 2022-10-25T19:43:35Z

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.

MichaelEischer

Please replace the options with a --group-accessible-repositories (maybe someone can come up with a better name). So far it looks like that option should cover the reported use cases and is not prone to accidental misuse as the current options.

directories and files add a changelog as requested by pull checklist make it go1.14 buildable expose command line option to configure umask for directories and files implement "group readable repos" instead of custom DirMode & FileMode

mlusetti · 2023-09-07T14:01:52Z

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

MichaelEischer

I'd prefer to grant the users groups both read and write access. By setting a corresponding umask it should still be possible to prevent those groups from actually being able to write to the repository storage.

regarding the option name @rawtaz suggested a few possible variants like: --group-access, --group-accessible, --group-accessible-repos or --group-repos. I'd also throw in an additional --group-accessible-data. What do you think?

MichaelEischer · 2023-09-11T19:42:48Z

repo/repo.go

-	}
-	if opt.FileMode == 0 {
-		opt.FileMode = DefaultFileMode
+	// if opt.dirMode == 0 {


I don't think we need these comments.

MichaelEischer · 2023-09-11T19:44:21Z

repo/repo.go

 const DefaultFileMode os.FileMode = 0600

+// GroupReadableDirMode is the file mode used for directory creation if group readable
+const GroupReadableDirMode os.FileMode = 0750


I'd prefer 0770 and 0660 an leave further restrictions to the umask used to run rest-server.

MichaelEischer · 2023-09-11T19:45:02Z

repo/repo.go

-	FsyncWarning   *sync.Once
+  FsyncWarning   *sync.Once
+
+  // If set, we will panic when an internal server error happens. This


That comment doesn't look like it belongs here.

eloo · 2024-03-29T09:59:32Z

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 · 2024-03-29T16:53:21Z

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

mlusetti · 2024-04-03T07:58:15Z

@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 · 2024-04-04T17:59:56Z

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.

fd0 force-pushed the filemode-dirmode-option-support branch from 408aa04 to 2624163 Compare April 15, 2022 08:28

MichaelEischer requested changes Sep 5, 2023

View reviewed changes

mlusetti added 3 commits September 7, 2023 15:51

expose command line option to configure umask for

63cf93d

directories and files add a changelog as requested by pull checklist make it go1.14 buildable expose command line option to configure umask for directories and files implement "group readable repos" instead of custom DirMode & FileMode

update changelog entry

265648e

typo

843b03c

mlusetti force-pushed the filemode-dirmode-option-support branch from ccbb37c to 843b03c Compare September 7, 2023 13:57

Merge branch 'master' into filemode-dirmode-option-support

6461f00

MichaelEischer reviewed Sep 11, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

mlusetti commented Apr 14, 2022 •

edited

mlusetti commented Apr 15, 2022

fd0 commented Apr 15, 2022

mlusetti commented Apr 15, 2022

fd0 commented Apr 15, 2022

fd0 commented Apr 15, 2022

mlusetti commented Apr 15, 2022

mlusetti commented Apr 15, 2022

mlusetti commented Apr 19, 2022

MichaelEischer commented Apr 20, 2022

mlusetti commented Apr 21, 2022

ianneub commented Oct 10, 2022

mlusetti commented Oct 25, 2022

rawtaz commented Oct 25, 2022

MichaelEischer left a comment

mlusetti commented Sep 7, 2023

MichaelEischer left a comment

MichaelEischer Sep 11, 2023

MichaelEischer Sep 11, 2023

MichaelEischer Sep 11, 2023

eloo commented Mar 29, 2024

MichaelEischer commented Mar 29, 2024

mlusetti commented Apr 3, 2024

MichaelEischer commented Apr 4, 2024

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

Are you sure you want to change the base?

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

Conversation

mlusetti commented Apr 14, 2022 • edited

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

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

Checklist

mlusetti commented Apr 15, 2022

fd0 commented Apr 15, 2022

mlusetti commented Apr 15, 2022

fd0 commented Apr 15, 2022

fd0 commented Apr 15, 2022

mlusetti commented Apr 15, 2022

mlusetti commented Apr 15, 2022

mlusetti commented Apr 19, 2022

MichaelEischer commented Apr 20, 2022

mlusetti commented Apr 21, 2022

ianneub commented Oct 10, 2022

mlusetti commented Oct 25, 2022

rawtaz commented Oct 25, 2022

MichaelEischer left a comment

Choose a reason for hiding this comment

mlusetti commented Sep 7, 2023

MichaelEischer left a comment

Choose a reason for hiding this comment

MichaelEischer Sep 11, 2023

Choose a reason for hiding this comment

MichaelEischer Sep 11, 2023

Choose a reason for hiding this comment

MichaelEischer Sep 11, 2023

Choose a reason for hiding this comment

eloo commented Mar 29, 2024

MichaelEischer commented Mar 29, 2024

mlusetti commented Apr 3, 2024

MichaelEischer commented Apr 4, 2024

mlusetti commented Apr 14, 2022 •

edited