-
Notifications
You must be signed in to change notification settings - Fork 1.7k
initial nfs_exports_info module #10122
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
base: main
Are you sure you want to change the base?
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your contribution!
plugins/modules/nfs_exports_info.py
Outdated
# SPDX-License-Identifier: GPL-3.0-or-later | ||
# Copyright: (c) 2025, Samaneh Yousefnezhad <[email protected]> | ||
# GNU General Public License v3.0+ | ||
# (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# SPDX-License-Identifier: GPL-3.0-or-later | |
# Copyright: (c) 2025, Samaneh Yousefnezhad <[email protected]> | |
# GNU General Public License v3.0+ | |
# (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) | |
#!/usr/bin/python | |
# Copyright (c) 2025, Samaneh Yousefnezhad <[email protected]> | |
# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) | |
# SPDX-License-Identifier: GPL-3.0-or-later |
@@ -0,0 +1,74 @@ | |||
# SPDX-License-Identifier: GPL-3.0-or-later |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# SPDX-License-Identifier: GPL-3.0-or-later | |
# Copyright (c) 2025, Samaneh Yousefnezhad <[email protected]> | |
# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) | |
# SPDX-License-Identifier: GPL-3.0-or-later |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this also applies for the module itself
plugins/modules/nfs_exports_info.py
Outdated
--- | ||
module: nfs_exports_info | ||
|
||
short_description: Extract folders, IPs, and options from /etc/exports |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
short_description: Extract folders, IPs, and options from /etc/exports | |
short_description: Extract folders, IPs, and options from C(/etc/exports) | |
version_added: 10.7.0 |
EXAMPLES = r""" | ||
- name: Get IPs and options per shared folder | ||
fava.infra.nfs_exports_info: | ||
output_format: 'ips_per_share' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would add a register
to make it more obvious that you should register the result:
output_format: 'ips_per_share' | |
output_format: 'ips_per_share' | |
register: result |
|
||
def get_exports(module, output_format, file_path="/etc/exports"): | ||
try: | ||
exports_file_digest = module.digest_from_file(file_path, 'sha1') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SHA1 is not always available. You need to do some fallback, or (better) provide at least a few different hash values based on availability. (I'd make the return value a dictionary with the key being the hash's name.)
mapping folders to their corresponding IP addresses and access options. | ||
|
||
author: | ||
- Samaneh Yousefnezhad (@yousefenzhad) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- Samaneh Yousefnezhad (@yousefenzhad) | |
- Samaneh Yousefnezhad (@yousefenzhad) | |
extends_documentation_fragment: | |
- community.general.attributes | |
- community.general.attributes.info_module |
# Add plugins/modules to path for direct import | ||
sys.path.insert(0, os.path.abspath( | ||
os.path.join(os.path.dirname(__file__), '../../../../plugins/modules') | ||
)) | ||
|
||
from nfs_exports_info import get_exports |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# Add plugins/modules to path for direct import | |
sys.path.insert(0, os.path.abspath( | |
os.path.join(os.path.dirname(__file__), '../../../../plugins/modules') | |
)) | |
from nfs_exports_info import get_exports | |
from ansible_collections.community.general.plugins.modules.nfs_exports_info import get_exports |
try: | ||
from unittest.mock import mock_open, patch, MagicMock | ||
except ImportError: | ||
from mock import mock_open, patch, MagicMock |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
try: | |
from unittest.mock import mock_open, patch, MagicMock | |
except ImportError: | |
from mock import mock_open, patch, MagicMock | |
from ansible_collections.community.internal_test_tools.tests.unit.compat.mock import mock_open, patch, MagicMock |
This comment was marked as outdated.
This comment was marked as outdated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hi @yousefenzhad thanks for your contribution!
I left some comments.
exports_info: | ||
description: | ||
- A mapping of shared folders to IPs and their options, or the reverse. | ||
- What it is depends on O(output_format). | ||
type: dict | ||
returned: always |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally, I think there should be two different return values, something like exports_per_ip
, exports_per_share
, and the format should be fixed for each one, instead of having a single return value with a variable format. The info
part is redundant, given the module name.
try: | ||
f = open(file_path, 'r') | ||
output_lines = f.readlines() | ||
f.close() | ||
except IOError: | ||
module.fail_json(msg="Could not read {}".format(file_path)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I may suggest, make this a small, simple function, say read_exports
or something similar, and that will simplify mocking/patching in tests/unit/plugins/modules/test_nfs_exports_info.py line 40
pattern = r'\s*(\S+)\s+(.+)' | ||
|
||
for line in output_lines: | ||
line = line.strip() | ||
if not line or line.startswith('#'): | ||
continue | ||
|
||
match = re.match(pattern, line) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Strongly recommend compiling the regexp outside of the loop:
pattern = r'\s*(\S+)\s+(.+)' | |
for line in output_lines: | |
line = line.strip() | |
if not line or line.startswith('#'): | |
continue | |
match = re.match(pattern, line) | |
pattern = re.compile(r'\s*(\S+)\s+(.+)') | |
for line in output_lines: | |
line = line.strip() | |
if not line or line.startswith('#'): | |
continue | |
match = pattern.match(line) |
folder = match.group(1) | ||
rest = match.group(2) | ||
|
||
entries = re.findall(r'(\d+\.\d+\.\d+\.\d+)\(([^)]+)\)', rest) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Compile it outside the loop as well, pls.
} | ||
|
||
except Exception as e: | ||
module.fail_json(msg="Error while processing exports: {}".format(str(e))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this is redundant, format()
will already convert it to a string.
module.fail_json(msg="Error while processing exports: {}".format(str(e))) | |
module.fail_json(msg="Error while processing exports: {}".format(e)) |
@felixfontein I am thinking this might be better off as a facts module, rather than just info. It is some system information, unlikely to change often (for most cases). |
That's fine with me. But in that case the |
SUMMARY
This pull request introduces a new Ansible module that parses the
/etc/exports
fileto extract NFS shared folder information, including export paths and associated options.
This is useful for automation tasks that require visibility into NFS export configurations.
ISSUE TYPE
COMPONENT NAME
nfs_exports_info
ADDITIONAL INFORMATION
This module reads and parses the
/etc/exports
file line by line, ignoring comments and blank lines,and returns a structured list of dictionaries with the following information for each export: