Skip to content
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

Add umask option for mount module #209

Open
wants to merge 14 commits into
base: main
Choose a base branch
from
3 changes: 3 additions & 0 deletions changelogs/fragments/209_add_mode_for_mount.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
---
minor_changes:
- mount - add ``mode`` parameter to mount module (https://github.com/ansible-collections/ansible.posix/issues/163).
satken2 marked this conversation as resolved.
Show resolved Hide resolved
24 changes: 24 additions & 0 deletions plugins/modules/mount.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,14 @@
the original file back if you somehow clobbered it incorrectly.
type: bool
default: no
umask:
description:
- The umask to set before creating new directory(ies) for the mount point.
If the mount point already exists, this parameter is not used.
- Note that after running this task and the device being successfully mounted,
the mode of the original directory will be hidden by the target device.
type: raw
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would change this to a 'strict string' to avoid all the 'octal/int' issues as we had to go through with mode in other modules.

version_added: '1.3.0'
notes:
- As of Ansible 2.3, the I(name) option has been changed to I(path) as
default, but I(name) still works as well.
Expand All @@ -122,6 +130,7 @@
fstype: iso9660
opts: ro,noauto
state: present
umask: 0022

- name: Mount up device by label
ansible.posix.mount:
Expand Down Expand Up @@ -665,6 +674,7 @@ def main():
src=dict(type='path'),
backup=dict(type='bool', default=False),
state=dict(type='str', required=True, choices=['absent', 'mounted', 'present', 'unmounted', 'remounted']),
umask=dict(type='raw'),
),
supports_check_mode=True,
required_if=(
Expand Down Expand Up @@ -761,6 +771,7 @@ def main():

state = module.params['state']
name = module.params['path']
umask = module.params['umask']
changed = False

if state == 'absent':
Expand Down Expand Up @@ -792,6 +803,16 @@ def main():
elif state == 'mounted':
dirs_created = []
if not os.path.exists(name) and not module.check_mode:
old_umask = None
if umask is not None:
if not isinstance(umask, int):
try:
umask = int(umask, 8)
except ValueError as e:
module.fail_json(msg="umask must be an octal integer: %s" % (to_native(e)))
old_umask = os.umask(umask)
os.umask(umask)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's the purpose of calling os.umask(umask) twice ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, it's my mistake.


try:
# Something like mkdir -p but with the possibility to undo.
# Based on some copy-paste from the "file" module.
Expand All @@ -816,6 +837,9 @@ def main():
except (OSError, IOError) as e:
module.fail_json(
msg="Error making dir %s: %s" % (name, to_native(e)))
finally:
if old_umask is not None:
os.umask(old_umask)

name, backup_lines, changed = _set_mount_save_old(module, args)
res = 0
Expand Down
76 changes: 76 additions & 0 deletions tests/integration/targets/mount/tasks/main.yml
Original file line number Diff line number Diff line change
Expand Up @@ -332,3 +332,79 @@
- /tmp/myfs.img
- /tmp/myfs
when: ansible_system in ('Linux')

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add a integration test which uses something like u+rw,g-wx,o-rwx or u=rw,g=r,o=r.

- name: Block to test umask option
block:
- name: Make sure that mount point does not exist
file:
path: /tmp/mount_dest
state: absent
- name: Create a directory to bind mount
file:
state: directory
path: /tmp/mount_source
- name: Bind mount a filesystem with umask
mount:
src: /tmp/mount_source
path: /tmp/mount_dest
state: mounted
fstype: None
opts: bind
umask: 0777
when: ansible_system != 'FreeBSD'
- name: Bind mount a filesystem with umask(FreeBSD)
mount:
src: /tmp/mount_source
path: /tmp/mount_dest
state: mounted
fstype: nullfs
umask: 0777
when: ansible_system == 'FreeBSD'
- name: Unmount FS to access underlying directory
command: |
umount /tmp/mount_dest
- name: Stat mount point directory
stat:
path: /tmp/mount_dest
register: mount_point_stat
- name: Assert that the mount point has right permission
assert:
that:
- mount_point_stat['stat']['mode'] == '0000'
- name: Cleanup directory
file:
path: /tmp/mount_dest
state: absent
- name: Bind mount a filesystem with string umask
mount:
src: /tmp/mount_source
path: /tmp/mount_dest
state: mounted
fstype: None
opts: bind
umask: "0777"
when: ansible_system != 'FreeBSD'
- name: Bind mount a filesystem with string umask(FreeBSD)
mount:
src: /tmp/mount_source
path: /tmp/mount_dest
state: mounted
fstype: nullfs
umask: "0777"
when: ansible_system == 'FreeBSD'
- name: Unmount FS to access underlying directory
command: |
umount /tmp/mount_dest
- name: Stat mount point directory
stat:
path: /tmp/mount_dest
register: mount_point_stat
- name: Assert that the mount point has right permission
assert:
that:
- mount_point_stat['stat']['mode'] == '0000'
- name: Remove the test FS
file:
path: /tmp/mount_dest
state: absent
when: ansible_system not in ('Darwin')