Add umask option for mount module

[satken2]

SUMMARY

I added mode option for mount module. When this module creates new directories for mount point, this value is used as permission of the directory.

Fixes #163, #178

ISSUE TYPE

• Feature Pull Request

COMPONENT NAME

• mount

ADDITIONAL INFORMATION

When we want to set specific permission to the mount point directory, we can use file module. However once the filesystem is mounted, the underlying directory is nearly inaccessible, and cannot be modified by file module. This is a bit inconvenient, so I added mode option so that permission can be set before actual mount operation.

In #163, author requests to add umask option but when the mount point does not exist, this module creates directories, not files. So I think mode is more appropriate than umask.

[Akasurde]

cc @quidame @aminvakil @justjais

[quidame]

Hi @satken2, thanks for your contribution !

IMHO, this PR doesn't fix #163, as said in its description: the new parameter only applies to the mountpoint itself, not its parents, even if they're created by the module, that means that if /srv is empty and the user wants to mount some device at /srv/foo/bar/public with a public access to it (say mode: u=rwx,go=rx), this mountpoint will still be unreachable by most users because /srv/foo and /srv/foo/bar got mode 0700.

[quidame]

In #163, author requests to add umask option but when the mount point does not exist, this module creates directories, not files. So I think mode is more appropriate than umask.

I think there is a confusion here. umask applies to both files and directories. To directories, with the exact value of the mask; to files, with the value of the mask and by zeroing all executable bits.

[satken2]

@quidame Thank you for your advice. Certainly, when creating directories recursively, I have to set permissions on all directories to resolve #163, as you point out. So I will fix my code.

I think there is a confusion here. umask applies to both files and directories. To directories, with the exact value of the mask; to files, with the value of the mask and by zeroing all executable bits.

The mount module only creates directories, so I don't think it is necessary to consider creating files, I guess. So I added mode option.

[quidame]

The mount module only creates directories, so I don't think it is necessary to consider creating files, I guess. So I added mode option.

There is no need to create files to use umask. It applies to almost any file (regular files, directories, unix sockets, named pipes or fifos, character devices...) except symlinks, at creation time. Even if at the end it doesn't change anything to your code, please consider umask as a plausible option. It seems that you have discarded it on a misunderstanding.

For me, mode sounds more like something I (user) would have control over, at any time, not only at creation time. (ok, in this case, not at any time, but this could be every time the underlying directory used as mountpoint is not mounted, and exposed/unhidden). At least 2 modules make use of umask (https://github.com/ansible/ansible/blob/ec408a69f19052190f1766a357c882fc86cfeada/lib/ansible/modules/git.py#L136-L141, https://github.com/ansible/ansible/blob/ec408a69f19052190f1766a357c882fc86cfeada/lib/ansible/modules/pip.py#L106-L114). I don't know if there are modules using mode in a one-shot way, that is the purpose of umask, and if there are, I would probably consider them as confusing, and still suggest: please consider umask for this new option. If a contributor needs mode to implement mode of the root of the mounted filesystem, and mode is already used for something else that behaves like umask rather than mode, what to do ?

[satken2]

@quidame Thank you for your polite explanation and suggestion. I have understood your intentions and changed my code to use umask instead of mode.

[verdurin]

I think this will solve the problem I recently saw when creating CephFS mounts with posix.mount, so I will be happy to see it merged.