puppet: refactored to use CmdRunner

.github/BOTMETA.yml

@@ -307,6 +307,9 @@ files:
   $module_utils/pipx.py:
     labels: pipx
     maintainers: russoz
+  $module_utils/puppet.py:
+    labels: puppet
+    maintainers: russoz
   $module_utils/pure.py:
     labels: pure pure_storage
     maintainers: $team_purestorage

changelogs/fragments/5612-puppet-cmd-runner.yml

@@ -0,0 +1,2 @@
+minor_changes:
+  - puppet - refactored module to use ``CmdRunner`` for executing ``puppet`` (https://github.com/ansible-collections/community.general/pull/5612).

plugins/module_utils/puppet.py

@@ -0,0 +1,114 @@
+# -*- coding: utf-8 -*-
+# Copyright (c) 2022, Alexei Znamensky <russoz@gmail.com>
+# 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
+
+from __future__ import absolute_import, division, print_function
+__metaclass__ = type
+
+
+import os
+
+from ansible_collections.community.general.plugins.module_utils.cmd_runner import CmdRunner, cmd_runner_fmt
+
+
+_PUPPET_PATH_PREFIX = ["/opt/puppetlabs/bin"]
+
+
+def get_facter_dir():
+    if os.getuid() == 0:
+        return '/etc/facter/facts.d'
+    else:
+        return os.path.expanduser('~/.facter/facts.d')
+
+
+def _puppet_cmd(module):
+    return module.get_bin_path("puppet", False, _PUPPET_PATH_PREFIX)
+
+
+# If the `timeout` CLI command feature is removed,
+# Then we could add this as a fixed param to `puppet_runner`
+def ensure_agent_enabled(module):
+    runner = CmdRunner(
+        module,
+        command="puppet",
+        path_prefix=_PUPPET_PATH_PREFIX,
+        arg_formats=dict(
+            _agent_disabled=cmd_runner_fmt.as_fixed(['config', 'print', 'agent_disabled_lockfile']),
+        ),
+        check_rc=False,
+    )
+
+    rc, stdout, stderr = runner("_agent_disabled").run()
+    if os.path.exists(stdout.strip()):
+        module.fail_json(
+            msg="Puppet agent is administratively disabled.",
+            disabled=True)
+    elif rc != 0:
+        module.fail_json(
+            msg="Puppet agent state could not be determined.")
+
+
+def puppet_runner(module):
+
+    # Keeping backward compatibility, allow for running with the `timeout` CLI command.
+    # If this can be replaced with ansible `timeout` parameter in playbook,
+    # then this function could be removed.
+    def _prepare_base_cmd():
+        _tout_cmd = module.get_bin_path("timeout", False)
+        if _tout_cmd:
+            cmd = ["timeout", "-s", "9", module.params["timeout"], _puppet_cmd(module)]
+        else:
+            cmd = ["puppet"]
+        return cmd
+
+    def noop_func(v):
+        _noop = cmd_runner_fmt.as_map({
+            True: "--noop",
+            False: "--no-noop",
+        })
+        return _noop(module.check_mode or v)
+
+    _logdest_map = {
+        "syslog": ["--logdest", "syslog"],
+        "all": ["--logdest", "syslog", "--logdest", "console"],
+    }
+
+    @cmd_runner_fmt.unpack_args
+    def execute_func(execute, manifest):
+        if execute:
+            return ["--execute", execute]
+        else:
+            return [manifest]
+
+    runner = CmdRunner(
+        module,
+        command=_prepare_base_cmd(),
+        path_prefix=_PUPPET_PATH_PREFIX,
+        arg_formats=dict(
+            _agent_fixed=cmd_runner_fmt.as_fixed([
+                "agent", "--onetime", "--no-daemonize", "--no-usecacheonfailure",
+                "--no-splay", "--detailed-exitcodes", "--verbose", "--color", "0",
+            ]),
+            _apply_fixed=cmd_runner_fmt.as_fixed(["apply", "--detailed-exitcodes"]),
+            puppetmaster=cmd_runner_fmt.as_opt_val("--server"),
+            show_diff=cmd_runner_fmt.as_bool("--show-diff"),
+            confdir=cmd_runner_fmt.as_opt_val("--confdir"),
+            environment=cmd_runner_fmt.as_opt_val("--environment"),
+            tags=cmd_runner_fmt.as_func(lambda v: ["--tags", ",".join(v)]),
+            certname=cmd_runner_fmt.as_opt_eq_val("--certname"),
+            noop=cmd_runner_fmt.as_func(noop_func),
+            use_srv_records=cmd_runner_fmt.as_map({
+                True: "--usr_srv_records",
+                False: "--no-usr_srv_records",
+            }),
+            logdest=cmd_runner_fmt.as_map(_logdest_map, default=[]),
+            modulepath=cmd_runner_fmt.as_opt_eq_val("--modulepath"),
+            _execute=cmd_runner_fmt.as_func(execute_func),
+            summarize=cmd_runner_fmt.as_bool("--summarize"),
+            debug=cmd_runner_fmt.as_bool("--debug"),
+            verbose=cmd_runner_fmt.as_bool("--verbose"),
+        ),
+        check_rc=False,
+    )
+    return runner

plugins/modules/puppet.py

@@ -152,15 +152,9 @@
 import os
 import stat
 
-from ansible.module_utils.basic import AnsibleModule
-from ansible.module_utils.six.moves import shlex_quote
-
+import ansible_collections.community.general.plugins.module_utils.puppet as puppet_utils
 
-def _get_facter_dir():
-    if os.getuid() == 0:
-        return '/etc/facter/facts.d'
-    else:
-        return os.path.expanduser('~/.facter/facts.d')
+from ansible.module_utils.basic import AnsibleModule
 
 
 def _write_structured_data(basedir, basename, data):
@@ -212,16 +206,6 @@ def main():
     )
     p = module.params
 
-    global PUPPET_CMD
-    PUPPET_CMD = module.get_bin_path("puppet", False, ['/opt/puppetlabs/bin'])
-
-    if not PUPPET_CMD:
-        module.fail_json(
-            msg="Could not find puppet. Please ensure it is installed.")
-
-    global TIMEOUT_CMD
-    TIMEOUT_CMD = module.get_bin_path("timeout", False)
-
     if p['manifest']:
         if not os.path.exists(p['manifest']):
             module.fail_json(
@@ -230,90 +214,24 @@ def main():
 
     # Check if puppet is disabled here
     if not p['manifest']:
-        rc, stdout, stderr = module.run_command(
-            PUPPET_CMD + " config print agent_disabled_lockfile")
-        if os.path.exists(stdout.strip()):
-            module.fail_json(
-                msg="Puppet agent is administratively disabled.",
-                disabled=True)
-        elif rc != 0:
-            module.fail_json(
-                msg="Puppet agent state could not be determined.")
+        puppet_utils.ensure_agent_enabled(module)
 
     if module.params['facts'] and not module.check_mode:
         _write_structured_data(
-            _get_facter_dir(),
+            puppet_utils.get_facter_dir(),
             module.params['facter_basename'],
             module.params['facts'])
 
-    if TIMEOUT_CMD:
-        base_cmd = "%(timeout_cmd)s -s 9 %(timeout)s %(puppet_cmd)s" % dict(
-            timeout_cmd=TIMEOUT_CMD,
-            timeout=shlex_quote(p['timeout']),
-            puppet_cmd=PUPPET_CMD)
-    else:
-        base_cmd = PUPPET_CMD
+    runner = puppet_utils.puppet_runner(module)
 
     if not p['manifest'] and not p['execute']:
-        cmd = ("%(base_cmd)s agent --onetime"
-               " --no-daemonize --no-usecacheonfailure --no-splay"
-               " --detailed-exitcodes --verbose --color 0") % dict(base_cmd=base_cmd)
-        if p['puppetmaster']:
-            cmd += " --server %s" % shlex_quote(p['puppetmaster'])
-        if p['show_diff']:
-            cmd += " --show_diff"
-        if p['confdir']:
-            cmd += " --confdir %s" % shlex_quote(p['confdir'])
-        if p['environment']:
-            cmd += " --environment '%s'" % p['environment']
-        if p['tags']:
-            cmd += " --tags '%s'" % ','.join(p['tags'])
-        if p['certname']:
-            cmd += " --certname='%s'" % p['certname']
-        if module.check_mode:
-            cmd += " --noop"
-        elif 'noop' in p:
-            if p['noop']:
-                cmd += " --noop"
-            else:
-                cmd += " --no-noop"
-        if p['use_srv_records'] is not None:
-            if not p['use_srv_records']:
-                cmd += " --no-use_srv_records"
-            else:
-                cmd += " --use_srv_records"
+        args_order = "_agent_fixed puppetmaster show_diff confdir environment tags certname noop use_srv_records"
+        with runner(args_order) as ctx:
+            rc, stdout, stderr = ctx.run()
     else:
-        cmd = "%s apply --detailed-exitcodes " % base_cmd
-        if p['logdest'] == 'syslog':
-            cmd += "--logdest syslog "
-        if p['logdest'] == 'all':
-            cmd += " --logdest syslog --logdest console"
-        if p['modulepath']:
-            cmd += "--modulepath='%s'" % p['modulepath']
-        if p['environment']:
-            cmd += "--environment '%s' " % p['environment']
-        if p['certname']:
-            cmd += " --certname='%s'" % p['certname']
-        if p['tags']:
-            cmd += " --tags '%s'" % ','.join(p['tags'])
-        if module.check_mode:
-            cmd += "--noop "
-        elif 'noop' in p:
-            if p['noop']:
-                cmd += " --noop"
-            else:
-                cmd += " --no-noop"
-        if p['execute']:
-            cmd += " --execute '%s'" % p['execute']
-        else:
-            cmd += " %s" % shlex_quote(p['manifest'])
-        if p['summarize']:
-            cmd += " --summarize"
-        if p['debug']:
-            cmd += " --debug"
-        if p['verbose']:
-            cmd += " --verbose"
-    rc, stdout, stderr = module.run_command(cmd)
+        args_order = "_apply_fixed logdest modulepath environment certname tags noop _execute summarize debug verbose"
+        with runner(args_order) as ctx:
+            rc, stdout, stderr = ctx.run(_execute=[p['execute'], p['manifest']])
 
     if rc == 0:
         # success
@@ -335,11 +253,11 @@ def main():
     elif rc == 124:
         # timeout
         module.exit_json(
-            rc=rc, msg="%s timed out" % cmd, stdout=stdout, stderr=stderr)
+            rc=rc, msg="%s timed out" % ctx.cmd, stdout=stdout, stderr=stderr)
     else:
         # failure
         module.fail_json(
-            rc=rc, msg="%s failed with return code: %d" % (cmd, rc),
+            rc=rc, msg="%s failed with return code: %d" % (ctx.cmd, rc),
             stdout=stdout, stderr=stderr)
 
 

tests/sanity/ignore-2.11.txt

@@ -18,8 +18,7 @@ plugins/modules/manageiq_provider.py validate-modules:undocumented-parameter
 plugins/modules/manageiq_tags.py validate-modules:parameter-state-invalid-choice
 plugins/modules/osx_defaults.py validate-modules:parameter-state-invalid-choice
 plugins/modules/parted.py validate-modules:parameter-state-invalid-choice
-plugins/modules/puppet.py use-argspec-type-path
-plugins/modules/puppet.py validate-modules:parameter-invalid  # invalid alias - removed in 7.0.0
+plugins/modules/puppet.py validate-modules:parameter-invalid                          # invalid alias - removed in 7.0.0
 plugins/modules/rax_files_objects.py use-argspec-type-path
 plugins/modules/rax_files.py validate-modules:parameter-state-invalid-choice
 plugins/modules/rax.py use-argspec-type-path # fix needed

tests/sanity/ignore-2.12.txt

@@ -13,8 +13,7 @@ plugins/modules/manageiq_provider.py validate-modules:undocumented-parameter
 plugins/modules/manageiq_tags.py validate-modules:parameter-state-invalid-choice
 plugins/modules/osx_defaults.py validate-modules:parameter-state-invalid-choice
 plugins/modules/parted.py validate-modules:parameter-state-invalid-choice
-plugins/modules/puppet.py use-argspec-type-path
-plugins/modules/puppet.py validate-modules:parameter-invalid  # invalid alias - removed in 7.0.0
+plugins/modules/puppet.py validate-modules:parameter-invalid                          # invalid alias - removed in 7.0.0
 plugins/modules/rax_files_objects.py use-argspec-type-path
 plugins/modules/rax_files.py validate-modules:parameter-state-invalid-choice
 plugins/modules/rax.py use-argspec-type-path # fix needed

tests/sanity/ignore-2.13.txt

@@ -13,8 +13,7 @@ plugins/modules/manageiq_provider.py validate-modules:undocumented-parameter
 plugins/modules/manageiq_tags.py validate-modules:parameter-state-invalid-choice
 plugins/modules/osx_defaults.py validate-modules:parameter-state-invalid-choice
 plugins/modules/parted.py validate-modules:parameter-state-invalid-choice
-plugins/modules/puppet.py use-argspec-type-path
-plugins/modules/puppet.py validate-modules:parameter-invalid  # invalid alias - removed in 7.0.0
+plugins/modules/puppet.py validate-modules:parameter-invalid                          # invalid alias - removed in 7.0.0
 plugins/modules/rax_files_objects.py use-argspec-type-path
 plugins/modules/rax_files.py validate-modules:parameter-state-invalid-choice
 plugins/modules/rax.py use-argspec-type-path # fix needed

tests/sanity/ignore-2.14.txt

@@ -14,8 +14,7 @@ plugins/modules/manageiq_provider.py validate-modules:undocumented-parameter
 plugins/modules/manageiq_tags.py validate-modules:parameter-state-invalid-choice
 plugins/modules/osx_defaults.py validate-modules:parameter-state-invalid-choice
 plugins/modules/parted.py validate-modules:parameter-state-invalid-choice
-plugins/modules/puppet.py use-argspec-type-path
-plugins/modules/puppet.py validate-modules:parameter-invalid  # invalid alias - removed in 7.0.0
+plugins/modules/puppet.py validate-modules:parameter-invalid                          # invalid alias - removed in 7.0.0
 plugins/modules/rax_files_objects.py use-argspec-type-path
 plugins/modules/rax_files.py validate-modules:parameter-state-invalid-choice
 plugins/modules/rax.py use-argspec-type-path # fix needed

tests/sanity/ignore-2.15.txt

@@ -14,8 +14,7 @@ plugins/modules/manageiq_provider.py validate-modules:undocumented-parameter
 plugins/modules/manageiq_tags.py validate-modules:parameter-state-invalid-choice
 plugins/modules/osx_defaults.py validate-modules:parameter-state-invalid-choice
 plugins/modules/parted.py validate-modules:parameter-state-invalid-choice
-plugins/modules/puppet.py use-argspec-type-path
-plugins/modules/puppet.py validate-modules:parameter-invalid  # invalid alias - removed in 7.0.0
+plugins/modules/puppet.py validate-modules:parameter-invalid                          # invalid alias - removed in 7.0.0
 plugins/modules/rax_files_objects.py use-argspec-type-path
 plugins/modules/rax_files.py validate-modules:parameter-state-invalid-choice
 plugins/modules/rax.py use-argspec-type-path # fix needed