File improvements-on-ansiblegate-module-354.patch of Package salt.26143
From b58056da2f5a12e3d614650904039c0655ce1221 Mon Sep 17 00:00:00 2001
From: Alexander Graul <agraul@suse.com>
Date: Tue, 18 Jan 2022 19:41:03 +0100
Subject: [PATCH] Improvements on "ansiblegate" module (#354)
* Allow collecting Ansible Inventory from a minion
* Prevent crashing if ansible-playbook doesn't return JSON
* Add new 'ansible.discover_playbooks' method
* Include custom inventory when discovering Ansible playbooks
* Enhance 'ansible.discover_playbooks' to accept a list of locations
* Remove unused constants from Ansible utils
* Avoid string concatenation to calculate extra cmd args
* Add unit test for ansible.targets
* Improve Ansible roster targetting
* Add tests for new ansiblegate module functions
* Fix issue dealing with ungrouped targets on inventory
* Enable ansible utils for ansible roster tests
* Remove unnecessary code from Ansible utils
* Fix pylint issue
* Fix issue in documentation
Fix issue parsing errors in ansiblegate state module
---
salt/modules/ansiblegate.py | 167 +++++++++++++++++-
salt/roster/ansible.py | 17 +-
salt/states/ansiblegate.py | 12 +-
salt/utils/ansible.py | 41 +++++
.../pytests/unit/modules/test_ansiblegate.py | 99 ++++++++++-
.../example_playbooks/example-playbook2/hosts | 7 +
.../example-playbook2/site.yml | 28 +++
.../playbooks/example_playbooks/playbook1.yml | 5 +
tests/unit/roster/test_ansible.py | 2 +-
9 files changed, 367 insertions(+), 11 deletions(-)
create mode 100644 salt/utils/ansible.py
create mode 100644 tests/unit/files/playbooks/example_playbooks/example-playbook2/hosts
create mode 100644 tests/unit/files/playbooks/example_playbooks/example-playbook2/site.yml
create mode 100644 tests/unit/files/playbooks/example_playbooks/playbook1.yml
diff --git a/salt/modules/ansiblegate.py b/salt/modules/ansiblegate.py
index 328d9b7b0a..f33be6a00e 100644
--- a/salt/modules/ansiblegate.py
+++ b/salt/modules/ansiblegate.py
@@ -17,6 +17,7 @@ any Ansible module to respond.
import fnmatch
import json
import logging
+import os
import subprocess
import sys
from tempfile import NamedTemporaryFile
@@ -365,7 +366,171 @@ def playbooks(
}
ret = __salt__["cmd.run_all"](**cmd_kwargs)
log.debug("Ansible Playbook Return: %s", ret)
- retdata = json.loads(ret["stdout"])
+ try:
+ retdata = json.loads(ret["stdout"])
+ except ValueError:
+ retdata = ret
if "retcode" in ret:
__context__["retcode"] = retdata["retcode"] = ret["retcode"]
return retdata
+
+
+def targets(**kwargs):
+ """
+ Return the inventory from an Ansible inventory_file
+
+ :param inventory:
+ The inventory file to read the inventory from. Default: "/etc/ansible/hosts"
+
+ :param yaml:
+ Return the inventory as yaml output. Default: False
+
+ :param export:
+ Return inventory as export format. Default: False
+
+ CLI Example:
+
+ .. code-block:: bash
+
+ salt 'ansiblehost' ansible.targets
+ salt 'ansiblehost' ansible.targets inventory=my_custom_inventory
+
+ """
+ return __utils__["ansible.targets"](**kwargs)
+
+
+def discover_playbooks(path=None,
+ locations=None,
+ playbook_extension=None,
+ hosts_filename=None,
+ syntax_check=False):
+ """
+ Discover Ansible playbooks stored under the given path or from multiple paths (locations)
+
+ This will search for files matching with the playbook file extension under the given
+ root path and will also look for files inside the first level of directories in this path.
+
+ The return of this function would be a dict like this:
+
+ .. code-block:: python
+
+ {
+ "/home/foobar/": {
+ "my_ansible_playbook.yml": {
+ "fullpath": "/home/foobar/playbooks/my_ansible_playbook.yml",
+ "custom_inventory": "/home/foobar/playbooks/hosts"
+ },
+ "another_playbook.yml": {
+ "fullpath": "/home/foobar/playbooks/another_playbook.yml",
+ "custom_inventory": "/home/foobar/playbooks/hosts"
+ },
+ "lamp_simple/site.yml": {
+ "fullpath": "/home/foobar/playbooks/lamp_simple/site.yml",
+ "custom_inventory": "/home/foobar/playbooks/lamp_simple/hosts"
+ },
+ "lamp_proxy/site.yml": {
+ "fullpath": "/home/foobar/playbooks/lamp_proxy/site.yml",
+ "custom_inventory": "/home/foobar/playbooks/lamp_proxy/hosts"
+ }
+ },
+ "/srv/playbooks/": {
+ "example_playbook/example.yml": {
+ "fullpath": "/srv/playbooks/example_playbook/example.yml",
+ "custom_inventory": "/srv/playbooks/example_playbook/hosts"
+ }
+ }
+ }
+
+ :param path:
+ Path to discover playbooks from.
+
+ :param locations:
+ List of paths to discover playbooks from.
+
+ :param playbook_extension:
+ File extension of playbooks file to search for. Default: "yml"
+
+ :param hosts_filename:
+ Filename of custom playbook inventory to search for. Default: "hosts"
+
+ :param syntax_check:
+ Skip playbooks that do not pass "ansible-playbook --syntax-check" validation. Default: False
+
+ :return:
+ The discovered playbooks under the given paths
+
+ CLI Example:
+
+ .. code-block:: bash
+
+ salt 'ansiblehost' ansible.discover_playbooks path=/srv/playbooks/
+ salt 'ansiblehost' ansible.discover_playbooks locations='["/srv/playbooks/", "/srv/foobar"]'
+
+ """
+
+ if not path and not locations:
+ raise CommandExecutionError("You have to specify either 'path' or 'locations' arguments")
+
+ if path and locations:
+ raise CommandExecutionError("You cannot specify 'path' and 'locations' at the same time")
+
+ if not playbook_extension:
+ playbook_extension = "yml"
+ if not hosts_filename:
+ hosts_filename = "hosts"
+
+ if path:
+ if not os.path.isabs(path):
+ raise CommandExecutionError("The given path is not an absolute path: {}".format(path))
+ if not os.path.isdir(path):
+ raise CommandExecutionError("The given path is not a directory: {}".format(path))
+ return {path: _explore_path(path, playbook_extension, hosts_filename, syntax_check)}
+
+ if locations:
+ all_ret = {}
+ for location in locations:
+ all_ret[location] = _explore_path(location, playbook_extension, hosts_filename, syntax_check)
+ return all_ret
+
+
+def _explore_path(path, playbook_extension, hosts_filename, syntax_check):
+ ret = {}
+
+ if not os.path.isabs(path):
+ log.error("The given path is not an absolute path: {}".format(path))
+ return ret
+ if not os.path.isdir(path):
+ log.error("The given path is not a directory: {}".format(path))
+ return ret
+
+ try:
+ # Check files in the given path
+ for _f in os.listdir(path):
+ _path = os.path.join(path, _f)
+ if os.path.isfile(_path) and _path.endswith("." + playbook_extension):
+ ret[_f] = {"fullpath": _path}
+ # Check for custom inventory file
+ if os.path.isfile(os.path.join(path, hosts_filename)):
+ ret[_f].update({"custom_inventory": os.path.join(path, hosts_filename)})
+ elif os.path.isdir(_path):
+ # Check files in the 1st level of subdirectories
+ for _f2 in os.listdir(_path):
+ _path2 = os.path.join(_path, _f2)
+ if os.path.isfile(_path2) and _path2.endswith("." + playbook_extension):
+ ret[os.path.join(_f, _f2)] = {"fullpath": _path2}
+ # Check for custom inventory file
+ if os.path.isfile(os.path.join(_path, hosts_filename)):
+ ret[os.path.join(_f, _f2)].update({"custom_inventory": os.path.join(_path, hosts_filename)})
+ except Exception as exc:
+ raise CommandExecutionError("There was an exception while discovering playbooks: {}".format(exc))
+
+ # Run syntax check validation
+ if syntax_check:
+ check_command = ["ansible-playbook", "--syntax-check"]
+ try:
+ for pb in list(ret):
+ if __salt__["cmd.retcode"](check_command + [ret[pb]]):
+ del ret[pb]
+ except Exception as exc:
+ raise CommandExecutionError("There was an exception while checking syntax of playbooks: {}".format(exc))
+ return ret
diff --git a/salt/roster/ansible.py b/salt/roster/ansible.py
index 7beaaf2075..d3b352de27 100644
--- a/salt/roster/ansible.py
+++ b/salt/roster/ansible.py
@@ -117,27 +117,32 @@ def targets(tgt, tgt_type="glob", **kwargs):
Return the targets from the ansible inventory_file
Default: /etc/salt/roster
"""
- inventory = __runner__["salt.cmd"](
- "cmd.run", "ansible-inventory -i {} --list".format(get_roster_file(__opts__))
- )
- __context__["inventory"] = __utils__["json.loads"](
- __utils__["stringutils.to_str"](inventory)
+ __context__["inventory"] = __utils__["ansible.targets"](
+ inventory=get_roster_file(__opts__), **kwargs
)
if tgt_type == "glob":
hosts = [
host for host in _get_hosts_from_group("all") if fnmatch.fnmatch(host, tgt)
]
+ elif tgt_type == "list":
+ hosts = [host for host in _get_hosts_from_group("all") if host in tgt]
elif tgt_type == "nodegroup":
hosts = _get_hosts_from_group(tgt)
+ else:
+ hosts = []
+
return {host: _get_hostvars(host) for host in hosts}
def _get_hosts_from_group(group):
inventory = __context__["inventory"]
+ if group not in inventory:
+ return []
hosts = [host for host in inventory[group].get("hosts", [])]
for child in inventory[group].get("children", []):
- if child != "ungrouped":
+ child_info = _get_hosts_from_group(child)
+ if child_info not in hosts:
hosts.extend(_get_hosts_from_group(child))
return hosts
diff --git a/salt/states/ansiblegate.py b/salt/states/ansiblegate.py
index 4afe6a020d..af5cb0f0e5 100644
--- a/salt/states/ansiblegate.py
+++ b/salt/states/ansiblegate.py
@@ -184,7 +184,11 @@ def playbooks(name, rundir=None, git_repo=None, git_kwargs=None, ansible_kwargs=
checks = __salt__["ansible.playbooks"](
name, rundir=rundir, check=True, diff=True, **ansible_kwargs
)
- if all(
+ if "stats" not in checks:
+ ret["comment"] = checks.get("stderr", checks)
+ ret["result"] = False
+ ret["changes"] = {}
+ elif all(
not check["changed"]
and not check["failures"]
and not check["unreachable"]
@@ -213,7 +217,11 @@ def playbooks(name, rundir=None, git_repo=None, git_kwargs=None, ansible_kwargs=
results = __salt__["ansible.playbooks"](
name, rundir=rundir, diff=True, **ansible_kwargs
)
- if all(
+ if "stats" not in results:
+ ret["comment"] = results.get("stderr", results)
+ ret["result"] = False
+ ret["changes"] = {}
+ elif all(
not check["changed"]
and not check["failures"]
and not check["unreachable"]
diff --git a/salt/utils/ansible.py b/salt/utils/ansible.py
new file mode 100644
index 0000000000..1e14037fd3
--- /dev/null
+++ b/salt/utils/ansible.py
@@ -0,0 +1,41 @@
+import logging
+import os
+
+# Import Salt libs
+import salt.utils.json
+import salt.utils.path
+import salt.utils.stringutils
+import salt.modules.cmdmod
+from salt.exceptions import CommandExecutionError
+
+__virtualname__ = "ansible"
+
+log = logging.getLogger(__name__)
+
+
+def __virtual__(): # pylint: disable=expected-2-blank-lines-found-0
+ if salt.utils.path.which("ansible-inventory"):
+ return __virtualname__
+ return (False, "Install `ansible` to use inventory")
+
+
+def targets(inventory="/etc/ansible/hosts", **kwargs):
+ """
+ Return the targets from the ansible inventory_file
+ Default: /etc/salt/roster
+ """
+ if not os.path.isfile(inventory):
+ raise CommandExecutionError("Inventory file not found: {}".format(inventory))
+
+ extra_cmd = []
+ if "export" in kwargs:
+ extra_cmd.append("--export")
+ if "yaml" in kwargs:
+ extra_cmd.append("--yaml")
+ inv = salt.modules.cmdmod.run(
+ "ansible-inventory -i {} --list {}".format(inventory, " ".join(extra_cmd))
+ )
+ if kwargs.get("yaml", False):
+ return salt.utils.stringutils.to_str(inv)
+ else:
+ return salt.utils.json.loads(salt.utils.stringutils.to_str(inv))
diff --git a/tests/pytests/unit/modules/test_ansiblegate.py b/tests/pytests/unit/modules/test_ansiblegate.py
index 44c9b12acb..f357133000 100644
--- a/tests/pytests/unit/modules/test_ansiblegate.py
+++ b/tests/pytests/unit/modules/test_ansiblegate.py
@@ -1,9 +1,15 @@
# Author: Bo Maryniuk <bo@suse.de>
+import os
import pytest
+
+import salt.config
+import salt.loader
import salt.modules.ansiblegate as ansiblegate
import salt.utils.json
+import salt.utils.path
from tests.support.mock import ANY, MagicMock, patch
+from tests.support.runtests import RUNTIME_VARS
pytestmark = [
pytest.mark.skip_on_windows(reason="Not supported on Windows"),
@@ -12,7 +18,7 @@ pytestmark = [
@pytest.fixture
def configure_loader_modules():
- return {ansiblegate: {}}
+ return {ansiblegate: {"__utils__": {}}}
def test_ansible_module_help():
@@ -133,3 +139,94 @@ def test_ansible_playbooks_return_retcode():
):
ret = ansiblegate.playbooks("fake-playbook.yml")
assert "retcode" in ret
+
+
+def test_ansible_targets():
+ """
+ Test ansible.targets execution module function.
+ :return:
+ """
+ ansible_inventory_ret = """
+{
+ "_meta": {
+ "hostvars": {
+ "uyuni-stable-ansible-centos7-1.tf.local": {
+ "ansible_ssh_private_key_file": "/etc/ansible/my_ansible_private_key"
+ },
+ "uyuni-stable-ansible-centos7-2.tf.local": {
+ "ansible_ssh_private_key_file": "/etc/ansible/my_ansible_private_key"
+ }
+ }
+ },
+ "all": {
+ "children": [
+ "ungrouped"
+ ]
+ },
+ "ungrouped": {
+ "hosts": [
+ "uyuni-stable-ansible-centos7-1.tf.local",
+ "uyuni-stable-ansible-centos7-2.tf.local"
+ ]
+ }
+}
+ """
+ ansible_inventory_mock = MagicMock(return_value=ansible_inventory_ret)
+ with patch("salt.utils.path.which", MagicMock(return_value=True)):
+ opts = salt.config.DEFAULT_MINION_OPTS.copy()
+ utils = salt.loader.utils(opts, whitelist=["ansible"])
+ with patch("salt.modules.cmdmod.run", ansible_inventory_mock), patch.dict(
+ ansiblegate.__utils__, utils
+ ), patch("os.path.isfile", MagicMock(return_value=True)):
+ ret = ansiblegate.targets()
+ assert ansible_inventory_mock.call_args
+ assert "_meta" in ret
+ assert "uyuni-stable-ansible-centos7-1.tf.local" in ret["_meta"]["hostvars"]
+ assert (
+ "ansible_ssh_private_key_file"
+ in ret["_meta"]["hostvars"]["uyuni-stable-ansible-centos7-1.tf.local"]
+ )
+ assert "all" in ret
+ assert len(ret["ungrouped"]["hosts"]) == 2
+
+
+def test_ansible_discover_playbooks_single_path():
+ playbooks_dir = os.path.join(
+ RUNTIME_VARS.TESTS_DIR, "unit/files/playbooks/example_playbooks/"
+ )
+ ret = ansiblegate.discover_playbooks(playbooks_dir)
+ assert playbooks_dir in ret
+ assert ret[playbooks_dir]["playbook1.yml"] == {
+ "fullpath": os.path.join(playbooks_dir, "playbook1.yml")
+ }
+ assert ret[playbooks_dir]["example-playbook2/site.yml"] == {
+ "fullpath": os.path.join(playbooks_dir, "example-playbook2/site.yml"),
+ "custom_inventory": os.path.join(playbooks_dir, "example-playbook2/hosts"),
+ }
+
+
+def test_ansible_discover_playbooks_single_path_using_parameters():
+ playbooks_dir = os.path.join(
+ RUNTIME_VARS.TESTS_DIR, "unit/files/playbooks/example_playbooks/"
+ )
+ ret = ansiblegate.discover_playbooks(
+ playbooks_dir, playbook_extension="foobar", hosts_filename="deadbeaf"
+ )
+ assert playbooks_dir in ret
+ assert ret[playbooks_dir] == {}
+
+
+def test_ansible_discover_playbooks_multiple_locations():
+ playbooks_dir = os.path.join(
+ RUNTIME_VARS.TESTS_DIR, "unit/files/playbooks/example_playbooks/"
+ )
+ ret = ansiblegate.discover_playbooks(locations=[playbooks_dir, "/tmp/foobar"])
+ assert playbooks_dir in ret
+ assert "/tmp/foobar" in ret
+ assert ret[playbooks_dir]["playbook1.yml"] == {
+ "fullpath": os.path.join(playbooks_dir, "playbook1.yml")
+ }
+ assert ret[playbooks_dir]["example-playbook2/site.yml"] == {
+ "fullpath": os.path.join(playbooks_dir, "example-playbook2/site.yml"),
+ "custom_inventory": os.path.join(playbooks_dir, "example-playbook2/hosts"),
+ }
diff --git a/tests/unit/files/playbooks/example_playbooks/example-playbook2/hosts b/tests/unit/files/playbooks/example_playbooks/example-playbook2/hosts
new file mode 100644
index 0000000000..75783285f6
--- /dev/null
+++ b/tests/unit/files/playbooks/example_playbooks/example-playbook2/hosts
@@ -0,0 +1,7 @@
+[databases]
+host1
+host2
+
+[webservers]
+host3
+host4
diff --git a/tests/unit/files/playbooks/example_playbooks/example-playbook2/site.yml b/tests/unit/files/playbooks/example_playbooks/example-playbook2/site.yml
new file mode 100644
index 0000000000..a64ebd5e18
--- /dev/null
+++ b/tests/unit/files/playbooks/example_playbooks/example-playbook2/site.yml
@@ -0,0 +1,28 @@
+---
+- name: update web servers
+ hosts: webservers
+ remote_user: root
+
+ tasks:
+ - name: ensure apache is at the latest version
+ yum:
+ name: httpd
+ state: latest
+ - name: write the apache config file
+ template:
+ src: /srv/httpd.j2
+ dest: /etc/httpd.conf
+
+- name: update db servers
+ hosts: databases
+ remote_user: root
+
+ tasks:
+ - name: ensure postgresql is at the latest version
+ yum:
+ name: postgresql
+ state: latest
+ - name: ensure that postgresql is started
+ service:
+ name: postgresql
+ state: started
diff --git a/tests/unit/files/playbooks/example_playbooks/playbook1.yml b/tests/unit/files/playbooks/example_playbooks/playbook1.yml
new file mode 100644
index 0000000000..e258a101e1
--- /dev/null
+++ b/tests/unit/files/playbooks/example_playbooks/playbook1.yml
@@ -0,0 +1,5 @@
+---
+- hosts: all
+ gather_facts: false
+ tasks:
+ - ping:
diff --git a/tests/unit/roster/test_ansible.py b/tests/unit/roster/test_ansible.py
index 7f1144454b..c4ab8b7639 100644
--- a/tests/unit/roster/test_ansible.py
+++ b/tests/unit/roster/test_ansible.py
@@ -63,7 +63,7 @@ class AnsibleRosterTestCase(TestCase, mixins.LoaderModuleMockMixin):
opts = salt.config.master_config(
os.path.join(RUNTIME_VARS.TMP_CONF_DIR, "master")
)
- utils = salt.loader.utils(opts, whitelist=["json", "stringutils"])
+ utils = salt.loader.utils(opts, whitelist=["json", "stringutils", "ansible"])
runner = salt.loader.runner(opts, utils=utils, whitelist=["salt"])
return {ansible: {"__utils__": utils, "__opts__": {}, "__runner__": runner}}
--
2.34.1
