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 option to mount module (https://github.com/ansible-collections/ansible.posix/issues/163).
satken2 marked this conversation as resolved.
Show resolved Hide resolved
17 changes: 17 additions & 0 deletions plugins/modules/mount.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,17 @@
the original file back if you somehow clobbered it incorrectly.
type: bool
default: no
mode:
description:
- The permission applied to create a new directory for mount point.
If the mount point already exists, this parameter is not used.
- This parameter only affects to the mount point itself.
If this module creates multiple directories recursively,
other directories follow the system's default umask.
- Note that after running this task and device being successfuly mounted,
the mode of the original directory will be hidden by the target device.
satken2 marked this conversation as resolved.
Show resolved Hide resolved
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.

required: false
satken2 marked this conversation as resolved.
Show resolved Hide resolved
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 +133,7 @@
fstype: iso9660
opts: ro,noauto
state: present
mode: 0755

- name: Mount up device by label
ansible.posix.mount:
Expand Down Expand Up @@ -665,6 +677,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']),
mode=dict(type='raw'),
),
supports_check_mode=True,
required_if=(
Expand Down Expand Up @@ -761,6 +774,7 @@ def main():

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

if state == 'absent':
Expand Down Expand Up @@ -813,6 +827,9 @@ def main():
if not (ex.errno == errno.EEXIST and os.path.isdir(b_curpath)):
raise

if mode is not None:
os.chmod(name, int(mode))
satken2 marked this conversation as resolved.
Show resolved Hide resolved

except (OSError, IOError) as e:
module.fail_json(
msg="Error making dir %s: %s" % (name, to_native(e)))
Expand Down
41 changes: 41 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,44 @@
- /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 mode option in Linux
block:
- name: Create empty file
community.general.filesize:
path: /tmp/myfs.img
size: 20M
- name: Format FS
community.general.filesystem:
fstype: ext3
dev: /tmp/myfs.img
- name: Make sure that mount point does not exist
file:
path: /tmp/myfs
state: absent
- name: Mount the FS to non existent directory with mode option
mount:
path: /tmp/myfs
src: /tmp/myfs.img
fstype: ext3
state: mounted
mode: 0000
- name: Unmount FS to access underlying directory
command: |
umount /tmp/myfs.img
- name: Check status of mount point
stat:
path: /tmp/myfs
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: '{{ item }}'
state: absent
loop:
- /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.

why this restriction ? Does the new mode parameter not support other OSes ?